From 7e70a63e615d399072c8b8c2054d8d61844240d6 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 2 Jul 2017 01:37:47 +0200 Subject: Throw errors when doc comments are added where they're unused --- src/libsyntax/parse/parser.rs | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) (limited to 'src/libsyntax/parse') diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index af9a198b983..047f4b979d9 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2131,14 +2131,14 @@ impl<'a> Parser<'a> { } else { Ok(self.mk_expr(span, ExprKind::Tup(es), attrs)) } - }, + } token::OpenDelim(token::Brace) => { return self.parse_block_expr(lo, BlockCheckMode::Default, attrs); - }, - token::BinOp(token::Or) | token::OrOr => { + } + token::BinOp(token::Or) | token::OrOr => { let lo = self.span; return self.parse_lambda_expr(lo, CaptureBy::Ref, attrs); - }, + } token::OpenDelim(token::Bracket) => { self.bump(); @@ -2387,7 +2387,6 @@ impl<'a> Parser<'a> { pub fn parse_block_expr(&mut self, lo: Span, blk_mode: BlockCheckMode, outer_attrs: ThinVec) -> PResult<'a, P> { - self.expect(&token::OpenDelim(token::Brace))?; let mut attrs = outer_attrs; @@ -2421,6 +2420,12 @@ impl<'a> Parser<'a> { expr.map(|mut expr| { attrs.extend::>(expr.attrs.into()); expr.attrs = attrs; + if if let Some(ref doc) = expr.attrs.iter().find(|x| x.is_sugared_doc) { + self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); + true + } else { false } { + return expr; + } match expr.node { ExprKind::If(..) | ExprKind::IfLet(..) => { if !expr.attrs.is_empty() { @@ -3105,6 +3110,9 @@ impl<'a> Parser<'a> { // `else` token already eaten pub fn parse_else_expr(&mut self) -> PResult<'a, P> { + if self.prev_token_kind == PrevTokenKind::DocComment { + return Err(self.span_fatal_err(self.span, Error::UselessDocComment)); + } if self.eat_keyword(keywords::If) { return self.parse_if_expr(ThinVec::new()); } else { @@ -3118,6 +3126,9 @@ impl<'a> Parser<'a> { span_lo: Span, mut attrs: ThinVec) -> PResult<'a, P> { // Parse: `for in ` + if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { + self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); + } let pat = self.parse_pat()?; self.expect_keyword(keywords::In)?; @@ -3133,6 +3144,9 @@ impl<'a> Parser<'a> { pub fn parse_while_expr(&mut self, opt_ident: Option, span_lo: Span, mut attrs: ThinVec) -> PResult<'a, P> { + if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { + self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); + } if self.token.is_keyword(keywords::Let) { return self.parse_while_let_expr(opt_ident, span_lo, attrs); } @@ -3161,6 +3175,9 @@ impl<'a> Parser<'a> { pub fn parse_loop_expr(&mut self, opt_ident: Option, span_lo: Span, mut attrs: ThinVec) -> PResult<'a, P> { + if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { + self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); + } let (iattrs, body) = self.parse_inner_attrs_and_block()?; attrs.extend(iattrs); let span = span_lo.to(body.span); @@ -3171,6 +3188,9 @@ impl<'a> Parser<'a> { pub fn parse_catch_expr(&mut self, span_lo: Span, mut attrs: ThinVec) -> PResult<'a, P> { + if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { + self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); + } let (iattrs, body) = self.parse_inner_attrs_and_block()?; attrs.extend(iattrs); Ok(self.mk_expr(span_lo.to(body.span), ExprKind::Catch(body), attrs)) @@ -3178,6 +3198,9 @@ impl<'a> Parser<'a> { // `match` token already eaten fn parse_match_expr(&mut self, mut attrs: ThinVec) -> PResult<'a, P> { + if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { + self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); + } let match_span = self.prev_span; let lo = self.prev_span; let discriminant = self.parse_expr_res(RESTRICTION_NO_STRUCT_LITERAL, @@ -3215,6 +3238,9 @@ impl<'a> Parser<'a> { maybe_whole!(self, NtArm, |x| x); let attrs = self.parse_outer_attributes()?; + if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { + self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); + } let pats = self.parse_pats()?; let guard = if self.eat_keyword(keywords::If) { Some(self.parse_expr()?) @@ -3669,6 +3695,9 @@ impl<'a> Parser<'a> { /// Parse a local variable declaration fn parse_local(&mut self, attrs: ThinVec) -> PResult<'a, P> { + if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { + self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); + } let lo = self.span; let pat = self.parse_pat()?; @@ -4158,6 +4187,8 @@ impl<'a> Parser<'a> { stmts.push(stmt); } else if self.token == token::Eof { break; + } else if let token::DocComment(_) = self.token { + return Err(self.span_fatal_err(self.span, Error::UselessDocComment)); } else { // Found only `;` or `}`. continue; -- cgit 1.4.1-3-g733a5 From 1cebf98e4c2654548e764e937e0b712220ffb600 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 16 Jul 2017 00:17:35 +0200 Subject: Make a lint instead --- src/librustc/hir/mod.rs | 7 ++++++ src/librustc_lint/builtin.rs | 40 +++++++++++++++++++++++++++++++ src/librustc_lint/lib.rs | 1 + src/libsyntax/ast.rs | 7 ++++++ src/libsyntax/parse/parser.rs | 32 ------------------------- src/test/compile-fail/useless_comment.rs | 28 ++++++++++++---------- src/test/compile-fail/useless_comment2.rs | 25 ------------------- src/test/compile-fail/useless_comment3.rs | 22 ----------------- 8 files changed, 71 insertions(+), 91 deletions(-) delete mode 100644 src/test/compile-fail/useless_comment2.rs delete mode 100644 src/test/compile-fail/useless_comment3.rs (limited to 'src/libsyntax/parse') diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 1b14caad3c8..a3a133daa09 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -892,6 +892,13 @@ impl Decl_ { DeclItem(_) => &[] } } + + pub fn is_local(&self) -> bool { + match *self { + Decl_::DeclLocal(_) => true, + _ => false, + } + } } /// represents one arm of a 'match' diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 02d68a41b4c..ca30ed4a536 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -722,6 +722,46 @@ impl EarlyLintPass for IllegalFloatLiteralPattern { } } +declare_lint! { + pub UNUSED_DOC_COMMENT, + Warn, + "detects doc comments that aren't used by rustdoc" +} + +#[derive(Copy, Clone)] +pub struct UnusedDocComment; + +impl LintPass for UnusedDocComment { + fn get_lints(&self) -> LintArray { + lint_array![UNUSED_DOC_COMMENT] + } +} + +impl UnusedDocComment { + fn warn_if_doc<'a, 'tcx, + I: Iterator, + C: LintContext<'tcx>>(&self, mut attrs: I, cx: &C) { + if let Some(attr) = attrs.find(|a| a.is_value_str() && a.check_name("doc")) { + cx.struct_span_lint(UNUSED_DOC_COMMENT, attr.span, "doc comment not used by rustdoc") + .emit(); + } + } +} + +impl EarlyLintPass for UnusedDocComment { + fn check_local(&mut self, cx: &EarlyContext, decl: &ast::Local) { + self.warn_if_doc(decl.attrs.iter(), cx); + } + + fn check_arm(&mut self, cx: &EarlyContext, arm: &ast::Arm) { + self.warn_if_doc(arm.attrs.iter(), cx); + } + + fn check_expr(&mut self, cx: &EarlyContext, expr: &ast::Expr) { + self.warn_if_doc(expr.attrs.iter(), cx); + } +} + declare_lint! { pub UNCONDITIONAL_RECURSION, Warn, diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 21dca7f6c61..83c00c178a0 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -111,6 +111,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { UnusedImportBraces, AnonymousParameters, IllegalFloatLiteralPattern, + UnusedDocComment, ); add_early_builtin_with_new!(sess, diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index f7d9d532062..df3f68fd1c6 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -718,6 +718,13 @@ impl Stmt { }; self } + + pub fn is_item(&self) -> bool { + match self.node { + StmtKind::Local(_) => true, + _ => false, + } + } } impl fmt::Debug for Stmt { diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 047f4b979d9..582f72e398d 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2420,12 +2420,6 @@ impl<'a> Parser<'a> { expr.map(|mut expr| { attrs.extend::>(expr.attrs.into()); expr.attrs = attrs; - if if let Some(ref doc) = expr.attrs.iter().find(|x| x.is_sugared_doc) { - self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); - true - } else { false } { - return expr; - } match expr.node { ExprKind::If(..) | ExprKind::IfLet(..) => { if !expr.attrs.is_empty() { @@ -3110,9 +3104,6 @@ impl<'a> Parser<'a> { // `else` token already eaten pub fn parse_else_expr(&mut self) -> PResult<'a, P> { - if self.prev_token_kind == PrevTokenKind::DocComment { - return Err(self.span_fatal_err(self.span, Error::UselessDocComment)); - } if self.eat_keyword(keywords::If) { return self.parse_if_expr(ThinVec::new()); } else { @@ -3126,9 +3117,6 @@ impl<'a> Parser<'a> { span_lo: Span, mut attrs: ThinVec) -> PResult<'a, P> { // Parse: `for in ` - if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { - self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); - } let pat = self.parse_pat()?; self.expect_keyword(keywords::In)?; @@ -3144,9 +3132,6 @@ impl<'a> Parser<'a> { pub fn parse_while_expr(&mut self, opt_ident: Option, span_lo: Span, mut attrs: ThinVec) -> PResult<'a, P> { - if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { - self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); - } if self.token.is_keyword(keywords::Let) { return self.parse_while_let_expr(opt_ident, span_lo, attrs); } @@ -3175,9 +3160,6 @@ impl<'a> Parser<'a> { pub fn parse_loop_expr(&mut self, opt_ident: Option, span_lo: Span, mut attrs: ThinVec) -> PResult<'a, P> { - if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { - self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); - } let (iattrs, body) = self.parse_inner_attrs_and_block()?; attrs.extend(iattrs); let span = span_lo.to(body.span); @@ -3188,9 +3170,6 @@ impl<'a> Parser<'a> { pub fn parse_catch_expr(&mut self, span_lo: Span, mut attrs: ThinVec) -> PResult<'a, P> { - if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { - self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); - } let (iattrs, body) = self.parse_inner_attrs_and_block()?; attrs.extend(iattrs); Ok(self.mk_expr(span_lo.to(body.span), ExprKind::Catch(body), attrs)) @@ -3198,9 +3177,6 @@ impl<'a> Parser<'a> { // `match` token already eaten fn parse_match_expr(&mut self, mut attrs: ThinVec) -> PResult<'a, P> { - if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { - self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); - } let match_span = self.prev_span; let lo = self.prev_span; let discriminant = self.parse_expr_res(RESTRICTION_NO_STRUCT_LITERAL, @@ -3238,9 +3214,6 @@ impl<'a> Parser<'a> { maybe_whole!(self, NtArm, |x| x); let attrs = self.parse_outer_attributes()?; - if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { - self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); - } let pats = self.parse_pats()?; let guard = if self.eat_keyword(keywords::If) { Some(self.parse_expr()?) @@ -3695,9 +3668,6 @@ impl<'a> Parser<'a> { /// Parse a local variable declaration fn parse_local(&mut self, attrs: ThinVec) -> PResult<'a, P> { - if let Some(doc) = attrs.iter().find(|x| x.is_sugared_doc) { - self.span_fatal_err(doc.span, Error::UselessDocComment).emit(); - } let lo = self.span; let pat = self.parse_pat()?; @@ -4187,8 +4157,6 @@ impl<'a> Parser<'a> { stmts.push(stmt); } else if self.token == token::Eof { break; - } else if let token::DocComment(_) = self.token { - return Err(self.span_fatal_err(self.span, Error::UselessDocComment)); } else { // Found only `;` or `}`. continue; diff --git a/src/test/compile-fail/useless_comment.rs b/src/test/compile-fail/useless_comment.rs index a32988aff12..bceec186120 100644 --- a/src/test/compile-fail/useless_comment.rs +++ b/src/test/compile-fail/useless_comment.rs @@ -8,19 +8,23 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -fn foo3() -> i32 { - let mut x = 12; - /// z //~ ERROR E0585 - while x < 1 { - /// x //~ ERROR E0585 - //~^ ERROR attributes on non-item statements and expressions are experimental - x += 1; +#![deny(unused_doc_comment)] + +fn foo() { + /// a //~ ERROR unused doc comment + let x = 12; + + /// b //~ ERROR unused doc comment + match x { + /// c //~ ERROR unused doc comment + 1 => {}, + _ => {} } - /// d //~ ERROR E0585 - return x; + + /// foo //~ ERROR unused doc comment + unsafe {} } fn main() { - /// e //~ ERROR E0585 - foo3(); -} + foo(); +} \ No newline at end of file diff --git a/src/test/compile-fail/useless_comment2.rs b/src/test/compile-fail/useless_comment2.rs deleted file mode 100644 index 52ac7b6a769..00000000000 --- a/src/test/compile-fail/useless_comment2.rs +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -fn foo() { - /// a //~ ERROR E0585 - let x = 12; - - /// b //~ ERROR E0585 - match x { - /// c //~ ERROR E0585 - 1 => {}, - _ => {} - } -} - -fn main() { - foo(); -} \ No newline at end of file diff --git a/src/test/compile-fail/useless_comment3.rs b/src/test/compile-fail/useless_comment3.rs deleted file mode 100644 index c26031b5eb6..00000000000 --- a/src/test/compile-fail/useless_comment3.rs +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -fn foo() { - let x = 13; - /// x //~ ERROR E0585 - if x == 12 { - /// y - println!("hello"); - } -} - -fn main() { - foo(); -} -- cgit 1.4.1-3-g733a5 From 9b2f7624ecb743e9db8e135113f396a7956623e7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 10 Jul 2017 17:44:46 -0700 Subject: syntax: Add `tokens: Option` to Item This commit adds a new field to the `Item` AST node in libsyntax to optionally contain the original token stream that the item itself was parsed from. This is currently `None` everywhere but is intended for use later with procedural macros. --- src/librustc_metadata/cstore_impl.rs | 1 + src/libsyntax/ast.rs | 6 ++++++ src/libsyntax/diagnostics/plugin.rs | 1 + src/libsyntax/ext/build.rs | 6 ++++-- src/libsyntax/ext/expand.rs | 1 + src/libsyntax/ext/placeholders.rs | 1 + src/libsyntax/fold.rs | 8 ++++++-- src/libsyntax/parse/parser.rs | 1 + src/libsyntax/std_inject.rs | 2 ++ src/libsyntax/test.rs | 16 +++++++++++----- src/libsyntax_ext/global_asm.rs | 1 + 11 files changed, 35 insertions(+), 9 deletions(-) (limited to 'src/libsyntax/parse') diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 5b0612ddab6..25079613e58 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -389,6 +389,7 @@ impl CrateStore for cstore::CStore { legacy: def.legacy, }), vis: ast::Visibility::Inherited, + tokens: None, }) } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 4fc73787353..bd26ab5bd35 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1812,6 +1812,12 @@ pub struct Item { pub node: ItemKind, pub vis: Visibility, pub span: Span, + + /// Original tokens this item was parsed from. This isn't necessarily + /// available for all items, although over time more and more items should + /// have this be `Some`. Right now this is primarily used for procedural + /// macros, notably custom attributes. + pub tokens: Option, } #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)] diff --git a/src/libsyntax/diagnostics/plugin.rs b/src/libsyntax/diagnostics/plugin.rs index 2a5de3c7382..855f4cd3557 100644 --- a/src/libsyntax/diagnostics/plugin.rs +++ b/src/libsyntax/diagnostics/plugin.rs @@ -236,6 +236,7 @@ pub fn expand_build_diagnostic_array<'cx>(ecx: &'cx mut ExtCtxt, ), vis: ast::Visibility::Public, span: span, + tokens: None, }) ])) } diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index e004f7354eb..de0538e38b3 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -979,7 +979,8 @@ impl<'a> AstBuilder for ExtCtxt<'a> { id: ast::DUMMY_NODE_ID, node: node, vis: ast::Visibility::Inherited, - span: span + span: span, + tokens: None, }) } @@ -1147,7 +1148,8 @@ impl<'a> AstBuilder for ExtCtxt<'a> { attrs: vec![], node: ast::ItemKind::Use(vp), vis: vis, - span: sp + span: sp, + tokens: None, }) } diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index f6d56557166..16c264e0f94 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -214,6 +214,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { ident: keywords::Invalid.ident(), id: ast::DUMMY_NODE_ID, vis: ast::Visibility::Public, + tokens: None, }))); match self.expand(krate_item).make_items().pop().map(P::unwrap) { diff --git a/src/libsyntax/ext/placeholders.rs b/src/libsyntax/ext/placeholders.rs index 4fb138d506a..9bea641b033 100644 --- a/src/libsyntax/ext/placeholders.rs +++ b/src/libsyntax/ext/placeholders.rs @@ -46,6 +46,7 @@ pub fn placeholder(kind: ExpansionKind, id: ast::NodeId) -> Expansion { ExpansionKind::Items => Expansion::Items(SmallVector::one(P(ast::Item { id: id, span: span, ident: ident, vis: vis, attrs: attrs, node: ast::ItemKind::Mac(mac_placeholder()), + tokens: None, }))), ExpansionKind::TraitItems => Expansion::TraitItems(SmallVector::one(ast::TraitItem { id: id, span: span, ident: ident, attrs: attrs, diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 8c616df858a..71802d0aa28 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -1000,6 +1000,7 @@ pub fn noop_fold_crate(Crate {module, attrs, span}: Crate, vis: ast::Visibility::Public, span: span, node: ast::ItemKind::Mod(module), + tokens: None, })).into_iter(); let (module, attrs, span) = match items.next() { @@ -1032,7 +1033,7 @@ pub fn noop_fold_item(i: P, folder: &mut T) -> SmallVector(Item {id, ident, attrs, node, vis, span}: Item, +pub fn noop_fold_item_simple(Item {id, ident, attrs, node, vis, span, tokens}: Item, folder: &mut T) -> Item { Item { id: folder.new_id(id), @@ -1040,7 +1041,10 @@ pub fn noop_fold_item_simple(Item {id, ident, attrs, node, vis, span} ident: folder.fold_ident(ident), attrs: fold_attrs(attrs, folder), node: folder.fold_item_kind(node), - span: folder.new_span(span) + span: folder.new_span(span), + tokens: tokens.map(|tokens| { + folder.fold_tts(tokens.into()).into() + }), } } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index d1591a219b3..4f8d85a8da4 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -4653,6 +4653,7 @@ impl<'a> Parser<'a> { node: node, vis: vis, span: span, + tokens: None, // TODO: fill this in }) } diff --git a/src/libsyntax/std_inject.rs b/src/libsyntax/std_inject.rs index a8a9ae556f1..d9ed96f293a 100644 --- a/src/libsyntax/std_inject.rs +++ b/src/libsyntax/std_inject.rs @@ -60,6 +60,7 @@ pub fn maybe_inject_crates_ref(mut krate: ast::Crate, alt_std_name: Option - folded.map(|ast::Item {id, ident, attrs, node, vis, span}| { + folded.map(|ast::Item {id, ident, attrs, node, vis, span, tokens}| { let allow_str = Symbol::intern("allow"); let dead_code_str = Symbol::intern("dead_code"); let word_vec = vec![attr::mk_list_word_item(dead_code_str)]; @@ -212,7 +212,8 @@ impl fold::Folder for EntryPointCleaner { .collect(), node: node, vis: vis, - span: span + span: span, + tokens: tokens, } }), EntryPointType::None | @@ -255,6 +256,7 @@ fn mk_reexport_mod(cx: &mut TestCtxt, node: ast::ItemKind::Mod(reexport_mod), vis: ast::Visibility::Public, span: DUMMY_SP, + tokens: None, })).pop().unwrap(); (it, sym) @@ -465,7 +467,8 @@ fn mk_std(cx: &TestCtxt) -> P { node: vi, attrs: vec![], vis: vis, - span: sp + span: sp, + tokens: None, }) } @@ -506,7 +509,8 @@ fn mk_main(cx: &mut TestCtxt) -> P { id: ast::DUMMY_NODE_ID, node: main, vis: ast::Visibility::Public, - span: sp + span: sp, + tokens: None, }) } @@ -536,6 +540,7 @@ fn mk_test_module(cx: &mut TestCtxt) -> (P, Option>) { node: item_, vis: ast::Visibility::Public, span: DUMMY_SP, + tokens: None, })).pop().unwrap(); let reexport = cx.reexport_test_harness_main.map(|s| { // building `use = __test::main` @@ -551,7 +556,8 @@ fn mk_test_module(cx: &mut TestCtxt) -> (P, Option>) { attrs: vec![], node: ast::ItemKind::Use(P(use_path)), vis: ast::Visibility::Inherited, - span: DUMMY_SP + span: DUMMY_SP, + tokens: None, })).pop().unwrap() }); diff --git a/src/libsyntax_ext/global_asm.rs b/src/libsyntax_ext/global_asm.rs index dc67e1c45f6..8b0bb8cb891 100644 --- a/src/libsyntax_ext/global_asm.rs +++ b/src/libsyntax_ext/global_asm.rs @@ -61,5 +61,6 @@ pub fn expand_global_asm<'cx>(cx: &'cx mut ExtCtxt, })), vis: ast::Visibility::Inherited, span: sp, + tokens: None, }))) } -- cgit 1.4.1-3-g733a5 From 6375b77ebb640001e9d076eec8601d926d2543f7 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Thu, 27 Jul 2017 13:37:35 +0900 Subject: Add Span to ast::WhereClause --- src/libsyntax/ast.rs | 2 ++ src/libsyntax/fold.rs | 5 +++-- src/libsyntax/parse/mod.rs | 1 + src/libsyntax/parse/parser.rs | 4 ++++ src/libsyntax/print/pprust.rs | 2 ++ src/libsyntax_ext/deriving/generic/ty.rs | 1 + 6 files changed, 13 insertions(+), 2 deletions(-) (limited to 'src/libsyntax/parse') diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index f7d9d532062..ff9266fb0cb 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -321,6 +321,7 @@ impl Default for Generics { where_clause: WhereClause { id: DUMMY_NODE_ID, predicates: Vec::new(), + span: DUMMY_SP, }, span: DUMMY_SP, } @@ -332,6 +333,7 @@ impl Default for Generics { pub struct WhereClause { pub id: NodeId, pub predicates: Vec, + pub span: Span, } /// A single predicate in a `where` clause diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index eaec1eef172..714f02969ec 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -736,14 +736,15 @@ pub fn noop_fold_generics(Generics {ty_params, lifetimes, where_claus } pub fn noop_fold_where_clause( - WhereClause {id, predicates}: WhereClause, + WhereClause {id, predicates, span}: WhereClause, fld: &mut T) -> WhereClause { WhereClause { id: fld.new_id(id), predicates: predicates.move_map(|predicate| { fld.fold_where_predicate(predicate) - }) + }), + span: span, } } diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index bd9a621c00c..3c44ca7f332 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -885,6 +885,7 @@ mod tests { where_clause: ast::WhereClause { id: ast::DUMMY_NODE_ID, predicates: Vec::new(), + span: syntax_pos::DUMMY_SP, }, span: syntax_pos::DUMMY_SP, }, diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 2cd84d202ff..9fb4f4813e9 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -4373,6 +4373,7 @@ impl<'a> Parser<'a> { where_clause: WhereClause { id: ast::DUMMY_NODE_ID, predicates: Vec::new(), + span: syntax_pos::DUMMY_SP, }, span: span_lo.to(self.prev_span), }) @@ -4440,11 +4441,13 @@ impl<'a> Parser<'a> { let mut where_clause = WhereClause { id: ast::DUMMY_NODE_ID, predicates: Vec::new(), + span: syntax_pos::DUMMY_SP, }; if !self.eat_keyword(keywords::Where) { return Ok(where_clause); } + let lo = self.prev_span; // This is a temporary future proofing. // @@ -4522,6 +4525,7 @@ impl<'a> Parser<'a> { } } + where_clause.span = lo.to(self.prev_span); Ok(where_clause) } diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index b052b2cdbbb..e9d11e73837 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1041,6 +1041,7 @@ impl<'a> State<'a> { where_clause: ast::WhereClause { id: ast::DUMMY_NODE_ID, predicates: Vec::new(), + span: syntax_pos::DUMMY_SP, }, span: syntax_pos::DUMMY_SP, }; @@ -2983,6 +2984,7 @@ impl<'a> State<'a> { where_clause: ast::WhereClause { id: ast::DUMMY_NODE_ID, predicates: Vec::new(), + span: syntax_pos::DUMMY_SP, }, span: syntax_pos::DUMMY_SP, }; diff --git a/src/libsyntax_ext/deriving/generic/ty.rs b/src/libsyntax_ext/deriving/generic/ty.rs index 9c89f99cbb5..f5ac1743920 100644 --- a/src/libsyntax_ext/deriving/generic/ty.rs +++ b/src/libsyntax_ext/deriving/generic/ty.rs @@ -216,6 +216,7 @@ fn mk_generics(lifetimes: Vec, ty_params: Vec, s where_clause: ast::WhereClause { id: ast::DUMMY_NODE_ID, predicates: Vec::new(), + span: span, }, span: span, } -- cgit 1.4.1-3-g733a5 From 4886ec86651a5eaae1ddc834a941842904a5db61 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 12 Jul 2017 09:50:05 -0700 Subject: syntax: Capture a `TokenStream` when parsing items This is then later used by `proc_macro` to generate a new `proc_macro::TokenTree` which preserves span information. Unfortunately this isn't a bullet-proof approach as it doesn't handle the case when there's still other attributes on the item, especially inner attributes. Despite this the intention here is to solve the primary use case for procedural attributes, attached to functions as outer attributes, likely bare. In this situation we should be able to now yield a lossless stream of tokens to preserve span information. --- src/libproc_macro/lib.rs | 63 +++++++++- src/libsyntax/ast.rs | 7 ++ src/libsyntax/ext/placeholders.rs | 2 + src/libsyntax/fold.rs | 13 ++- src/libsyntax/parse/mod.rs | 9 +- src/libsyntax/parse/parser.rs | 130 ++++++++++++++++++++- src/libsyntax_ext/deriving/generic/mod.rs | 2 + .../proc-macro/attribute-with-error.rs | 32 ++++- .../proc-macro/attributes-included.rs | 30 +++++ .../proc-macro/auxiliary/attributes-included.rs | 130 +++++++++++++++++++++ 10 files changed, 398 insertions(+), 20 deletions(-) create mode 100644 src/test/compile-fail-fulldeps/proc-macro/attributes-included.rs create mode 100644 src/test/compile-fail-fulldeps/proc-macro/auxiliary/attributes-included.rs (limited to 'src/libsyntax/parse') diff --git a/src/libproc_macro/lib.rs b/src/libproc_macro/lib.rs index 0f0d4138062..1bffffd6c9e 100644 --- a/src/libproc_macro/lib.rs +++ b/src/libproc_macro/lib.rs @@ -510,15 +510,38 @@ impl TokenTree { Literal(..) | DocComment(..) => TokenNode::Literal(self::Literal(token)), Interpolated(ref nt) => { - let mut node = None; - if let Nonterminal::NtItem(ref item) = nt.0 { - if let Some(ref tokens) = item.tokens { - node = Some(TokenNode::Group(Delimiter::None, - TokenStream(tokens.clone()))); + // An `Interpolated` token means that we have a `Nonterminal` + // which is often a parsed AST item. At this point we now need + // to convert the parsed AST to an actual token stream, e.g. + // un-parse it basically. + // + // Unfortunately there's not really a great way to do that in a + // guaranteed lossless fashion right now. The fallback here is + // to just stringify the AST node and reparse it, but this loses + // all span information. + // + // As a result, some AST nodes are annotated with the token + // stream they came from. Attempt to extract these lossless + // token streams before we fall back to the stringification. + let mut tokens = None; + + match nt.0 { + Nonterminal::NtItem(ref item) => { + tokens = prepend_attrs(&item.attrs, item.tokens.as_ref(), span); } + Nonterminal::NtTraitItem(ref item) => { + tokens = prepend_attrs(&item.attrs, item.tokens.as_ref(), span); + } + Nonterminal::NtImplItem(ref item) => { + tokens = prepend_attrs(&item.attrs, item.tokens.as_ref(), span); + } + _ => {} } - node.unwrap_or_else(|| { + tokens.map(|tokens| { + TokenNode::Group(Delimiter::None, + TokenStream(tokens.clone())) + }).unwrap_or_else(|| { __internal::with_sess(|(sess, _)| { TokenNode::Group(Delimiter::None, TokenStream(nt.1.force(|| { // FIXME(jseyfried): Avoid this pretty-print + reparse hack @@ -592,6 +615,34 @@ impl TokenTree { } } +fn prepend_attrs(attrs: &[ast::Attribute], + tokens: Option<&tokenstream::TokenStream>, + span: syntax_pos::Span) + -> Option +{ + let tokens = match tokens { + Some(tokens) => tokens, + None => return None, + }; + if attrs.len() == 0 { + return Some(tokens.clone()) + } + let mut builder = tokenstream::TokenStreamBuilder::new(); + for attr in attrs { + assert_eq!(attr.style, ast::AttrStyle::Outer, + "inner attributes should prevent cached tokens from existing"); + let stream = __internal::with_sess(|(sess, _)| { + // FIXME: Avoid this pretty-print + reparse hack as bove + let name = "".to_owned(); + let source = pprust::attr_to_string(attr); + parse_stream_from_source_str(name, source, sess, Some(span)) + }); + builder.push(stream); + } + builder.push(tokens.clone()); + Some(builder.build()) +} + /// Permanently unstable internal implementation details of this crate. This /// should not be used. /// diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index bd26ab5bd35..fb791541524 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1149,6 +1149,8 @@ pub struct TraitItem { pub attrs: Vec, pub node: TraitItemKind, pub span: Span, + /// See `Item::tokens` for what this is + pub tokens: Option, } #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)] @@ -1168,6 +1170,8 @@ pub struct ImplItem { pub attrs: Vec, pub node: ImplItemKind, pub span: Span, + /// See `Item::tokens` for what this is + pub tokens: Option, } #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)] @@ -1817,6 +1821,9 @@ pub struct Item { /// available for all items, although over time more and more items should /// have this be `Some`. Right now this is primarily used for procedural /// macros, notably custom attributes. + /// + /// Note that the tokens here do not include the outer attributes, but will + /// include inner attributes. pub tokens: Option, } diff --git a/src/libsyntax/ext/placeholders.rs b/src/libsyntax/ext/placeholders.rs index 9bea641b033..e3377c1d8de 100644 --- a/src/libsyntax/ext/placeholders.rs +++ b/src/libsyntax/ext/placeholders.rs @@ -51,11 +51,13 @@ pub fn placeholder(kind: ExpansionKind, id: ast::NodeId) -> Expansion { ExpansionKind::TraitItems => Expansion::TraitItems(SmallVector::one(ast::TraitItem { id: id, span: span, ident: ident, attrs: attrs, node: ast::TraitItemKind::Macro(mac_placeholder()), + tokens: None, })), ExpansionKind::ImplItems => Expansion::ImplItems(SmallVector::one(ast::ImplItem { id: id, span: span, ident: ident, vis: vis, attrs: attrs, node: ast::ImplItemKind::Macro(mac_placeholder()), defaultness: ast::Defaultness::Final, + tokens: None, })), ExpansionKind::Pat => Expansion::Pat(P(ast::Pat { id: id, span: span, node: ast::PatKind::Mac(mac_placeholder()), diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 71802d0aa28..279f63d13a4 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -957,7 +957,8 @@ pub fn noop_fold_trait_item(i: TraitItem, folder: &mut T) TraitItemKind::Macro(folder.fold_mac(mac)) } }, - span: folder.new_span(i.span) + span: folder.new_span(i.span), + tokens: i.tokens, }) } @@ -980,7 +981,8 @@ pub fn noop_fold_impl_item(i: ImplItem, folder: &mut T) ast::ImplItemKind::Type(ty) => ast::ImplItemKind::Type(folder.fold_ty(ty)), ast::ImplItemKind::Macro(mac) => ast::ImplItemKind::Macro(folder.fold_mac(mac)) }, - span: folder.new_span(i.span) + span: folder.new_span(i.span), + tokens: i.tokens, }) } @@ -1042,9 +1044,10 @@ pub fn noop_fold_item_simple(Item {id, ident, attrs, node, vis, span, attrs: fold_attrs(attrs, folder), node: folder.fold_item_kind(node), span: folder.new_span(span), - tokens: tokens.map(|tokens| { - folder.fold_tts(tokens.into()).into() - }), + + // FIXME: if this is replaced with a call to `folder.fold_tts` it causes + // an ICE during resolve... odd! + tokens: tokens, } } diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index bd9a621c00c..45e0b8404cc 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -843,11 +843,18 @@ mod tests { // check the contents of the tt manually: #[test] fn parse_fundecl () { // this test depends on the intern order of "fn" and "i32" - assert_eq!(string_to_item("fn a (b : i32) { b; }".to_string()), + let item = string_to_item("fn a (b : i32) { b; }".to_string()).map(|m| { + m.map(|mut m| { + m.tokens = None; + m + }) + }); + assert_eq!(item, Some( P(ast::Item{ident:Ident::from_str("a"), attrs:Vec::new(), id: ast::DUMMY_NODE_ID, + tokens: None, node: ast::ItemKind::Fn(P(ast::FnDecl { inputs: vec![ast::Arg{ ty: P(ast::Ty{id: ast::DUMMY_NODE_ID, diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 4f8d85a8da4..1a10aa9d621 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -216,6 +216,30 @@ struct TokenCursorFrame { open_delim: bool, tree_cursor: tokenstream::Cursor, close_delim: bool, + last_token: LastToken, +} + +/// This is used in `TokenCursorFrame` above to track tokens that are consumed +/// by the parser, and then that's transitively used to record the tokens that +/// each parse AST item is created with. +/// +/// Right now this has two states, either collecting tokens or not collecting +/// tokens. If we're collecting tokens we just save everything off into a local +/// `Vec`. This should eventually though likely save tokens from the original +/// token stream and just use slicing of token streams to avoid creation of a +/// whole new vector. +/// +/// The second state is where we're passively not recording tokens, but the last +/// token is still tracked for when we want to start recording tokens. This +/// "last token" means that when we start recording tokens we'll want to ensure +/// that this, the first token, is included in the output. +/// +/// You can find some more example usage of this in the `collect_tokens` method +/// on the parser. +#[derive(Clone)] +enum LastToken { + Collecting(Vec), + Was(Option), } impl TokenCursorFrame { @@ -226,6 +250,7 @@ impl TokenCursorFrame { open_delim: delimited.delim == token::NoDelim, tree_cursor: delimited.stream().into_trees(), close_delim: delimited.delim == token::NoDelim, + last_token: LastToken::Was(None), } } } @@ -250,6 +275,11 @@ impl TokenCursor { return TokenAndSpan { tok: token::Eof, sp: syntax_pos::DUMMY_SP } }; + match self.frame.last_token { + LastToken::Collecting(ref mut v) => v.push(tree.clone()), + LastToken::Was(ref mut t) => *t = Some(tree.clone()), + } + match tree { TokenTree::Token(sp, tok) => return TokenAndSpan { tok: tok, sp: sp }, TokenTree::Delimited(sp, ref delimited) => { @@ -1209,7 +1239,20 @@ impl<'a> Parser<'a> { /// Parse the items in a trait declaration pub fn parse_trait_item(&mut self, at_end: &mut bool) -> PResult<'a, TraitItem> { maybe_whole!(self, NtTraitItem, |x| x); - let mut attrs = self.parse_outer_attributes()?; + let attrs = self.parse_outer_attributes()?; + let (mut item, tokens) = self.collect_tokens(|this| { + this.parse_trait_item_(at_end, attrs) + })?; + // See `parse_item` for why this clause is here. + if !item.attrs.iter().any(|attr| attr.style == AttrStyle::Inner) { + item.tokens = Some(tokens); + } + Ok(item) + } + + fn parse_trait_item_(&mut self, + at_end: &mut bool, + mut attrs: Vec) -> PResult<'a, TraitItem> { let lo = self.span; let (name, node) = if self.eat_keyword(keywords::Type) { @@ -1304,6 +1347,7 @@ impl<'a> Parser<'a> { attrs: attrs, node: node, span: lo.to(self.prev_span), + tokens: None, }) } @@ -4653,7 +4697,7 @@ impl<'a> Parser<'a> { node: node, vis: vis, span: span, - tokens: None, // TODO: fill this in + tokens: None, }) } @@ -4709,8 +4753,21 @@ impl<'a> Parser<'a> { /// Parse an impl item. pub fn parse_impl_item(&mut self, at_end: &mut bool) -> PResult<'a, ImplItem> { maybe_whole!(self, NtImplItem, |x| x); + let attrs = self.parse_outer_attributes()?; + let (mut item, tokens) = self.collect_tokens(|this| { + this.parse_impl_item_(at_end, attrs) + })?; + + // See `parse_item` for why this clause is here. + if !item.attrs.iter().any(|attr| attr.style == AttrStyle::Inner) { + item.tokens = Some(tokens); + } + Ok(item) + } - let mut attrs = self.parse_outer_attributes()?; + fn parse_impl_item_(&mut self, + at_end: &mut bool, + mut attrs: Vec) -> PResult<'a, ImplItem> { let lo = self.span; let vis = self.parse_visibility(false)?; let defaultness = self.parse_defaultness()?; @@ -4742,7 +4799,8 @@ impl<'a> Parser<'a> { vis: vis, defaultness: defaultness, attrs: attrs, - node: node + node: node, + tokens: None, }) } @@ -6018,9 +6076,71 @@ impl<'a> Parser<'a> { Ok(None) } + fn collect_tokens(&mut self, f: F) -> PResult<'a, (R, TokenStream)> + where F: FnOnce(&mut Self) -> PResult<'a, R> + { + // Record all tokens we parse when parsing this item. + let mut tokens = Vec::new(); + match self.token_cursor.frame.last_token { + LastToken::Collecting(_) => { + panic!("cannot collect tokens recursively yet") + } + LastToken::Was(ref mut last) => tokens.extend(last.take()), + } + self.token_cursor.frame.last_token = LastToken::Collecting(tokens); + let prev = self.token_cursor.stack.len(); + let ret = f(self); + let last_token = if self.token_cursor.stack.len() == prev { + &mut self.token_cursor.frame.last_token + } else { + &mut self.token_cursor.stack[prev].last_token + }; + let mut tokens = match *last_token { + LastToken::Collecting(ref mut v) => mem::replace(v, Vec::new()), + LastToken::Was(_) => panic!("our vector went away?"), + }; + + // If we're not at EOF our current token wasn't actually consumed by + // `f`, but it'll still be in our list that we pulled out. In that case + // put it back. + if self.token == token::Eof { + *last_token = LastToken::Was(None); + } else { + *last_token = LastToken::Was(tokens.pop()); + } + + Ok((ret?, tokens.into_iter().collect())) + } + pub fn parse_item(&mut self) -> PResult<'a, Option>> { let attrs = self.parse_outer_attributes()?; - self.parse_item_(attrs, true, false) + + let (ret, tokens) = self.collect_tokens(|this| { + this.parse_item_(attrs, true, false) + })?; + + // Once we've parsed an item and recorded the tokens we got while + // parsing we may want to store `tokens` into the item we're about to + // return. Note, though, that we specifically didn't capture tokens + // related to outer attributes. The `tokens` field here may later be + // used with procedural macros to convert this item back into a token + // stream, but during expansion we may be removing attributes as we go + // along. + // + // If we've got inner attributes then the `tokens` we've got above holds + // these inner attributes. If an inner attribute is expanded we won't + // actually remove it from the token stream, so we'll just keep yielding + // it (bad!). To work around this case for now we just avoid recording + // `tokens` if we detect any inner attributes. This should help keep + // expansion correct, but we should fix this bug one day! + Ok(ret.map(|item| { + item.map(|mut i| { + if !i.attrs.iter().any(|attr| attr.style == AttrStyle::Inner) { + i.tokens = Some(tokens); + } + i + }) + })) } fn parse_path_list_items(&mut self) -> PResult<'a, Vec> { diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 4acd65bbf86..3cbc7938bde 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -504,6 +504,7 @@ impl<'a> TraitDef<'a> { defaultness: ast::Defaultness::Final, attrs: Vec::new(), node: ast::ImplItemKind::Type(type_def.to_ty(cx, self.span, type_ident, generics)), + tokens: None, } }); @@ -930,6 +931,7 @@ impl<'a> MethodDef<'a> { decl: fn_decl, }, body_block), + tokens: None, } } diff --git a/src/test/compile-fail-fulldeps/proc-macro/attribute-with-error.rs b/src/test/compile-fail-fulldeps/proc-macro/attribute-with-error.rs index a74343cda86..65f4b6350c4 100644 --- a/src/test/compile-fail-fulldeps/proc-macro/attribute-with-error.rs +++ b/src/test/compile-fail-fulldeps/proc-macro/attribute-with-error.rs @@ -14,12 +14,38 @@ extern crate attribute_with_error; -#[attribute_with_error::foo] -fn test() { +use attribute_with_error::foo; + +#[foo] +fn test1() { let a: i32 = "foo"; //~^ ERROR: mismatched types } +fn test2() { + #![foo] + + // FIXME: should have a type error here and assert it works but it doesn't +} + +trait A { + // FIXME: should have a #[foo] attribute here and assert that it works + fn foo(&self) { + let a: i32 = "foo"; + //~^ ERROR: mismatched types + } +} + +struct B; + +impl A for B { + #[foo] + fn foo(&self) { + let a: i32 = "foo"; + //~^ ERROR: mismatched types + } +} + +#[foo] fn main() { - test(); } diff --git a/src/test/compile-fail-fulldeps/proc-macro/attributes-included.rs b/src/test/compile-fail-fulldeps/proc-macro/attributes-included.rs new file mode 100644 index 00000000000..508f8dac571 --- /dev/null +++ b/src/test/compile-fail-fulldeps/proc-macro/attributes-included.rs @@ -0,0 +1,30 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// aux-build:attributes-included.rs + +#![feature(proc_macro, rustc_attrs)] + +extern crate attributes_included; + +#[attributes_included::bar] +#[inline] +/// doc +#[attributes_included::foo] +#[inline] +/// doc +fn foo() { + let a: i32 = "foo"; //~ WARN: unused variable +} + +#[rustc_error] +fn main() { //~ ERROR: compilation successful + foo() +} diff --git a/src/test/compile-fail-fulldeps/proc-macro/auxiliary/attributes-included.rs b/src/test/compile-fail-fulldeps/proc-macro/auxiliary/attributes-included.rs new file mode 100644 index 00000000000..a1efbb88a4d --- /dev/null +++ b/src/test/compile-fail-fulldeps/proc-macro/auxiliary/attributes-included.rs @@ -0,0 +1,130 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// force-host +// no-prefer-dynamic + +#![feature(proc_macro)] +#![crate_type = "proc-macro"] + +extern crate proc_macro; + +use proc_macro::{TokenStream, TokenTree, TokenNode, Delimiter, Literal}; + +#[proc_macro_attribute] +pub fn foo(attr: TokenStream, input: TokenStream) -> TokenStream { + assert!(attr.is_empty()); + let input = input.into_iter().collect::>(); + { + let mut cursor = &input[..]; + assert_inline(&mut cursor); + assert_doc(&mut cursor); + assert_inline(&mut cursor); + assert_doc(&mut cursor); + assert_foo(&mut cursor); + assert!(cursor.is_empty()); + } + fold_stream(input.into_iter().collect()) +} + +#[proc_macro_attribute] +pub fn bar(attr: TokenStream, input: TokenStream) -> TokenStream { + assert!(attr.is_empty()); + let input = input.into_iter().collect::>(); + { + let mut cursor = &input[..]; + assert_inline(&mut cursor); + assert_doc(&mut cursor); + assert_invoc(&mut cursor); + assert_inline(&mut cursor); + assert_doc(&mut cursor); + assert_foo(&mut cursor); + assert!(cursor.is_empty()); + } + input.into_iter().collect() +} + +fn assert_inline(slice: &mut &[TokenTree]) { + match slice[0].kind { + TokenNode::Op('#', _) => {} + _ => panic!("expected '#' char"), + } + match slice[1].kind { + TokenNode::Group(Delimiter::Bracket, _) => {} + _ => panic!("expected brackets"), + } + *slice = &slice[2..]; +} + +fn assert_doc(slice: &mut &[TokenTree]) { + match slice[0].kind { + TokenNode::Literal(_) => {} + _ => panic!("expected literal doc comment got other"), + } + *slice = &slice[1..]; +} + +fn assert_invoc(slice: &mut &[TokenTree]) { + match slice[0].kind { + TokenNode::Op('#', _) => {} + _ => panic!("expected '#' char"), + } + match slice[1].kind { + TokenNode::Group(Delimiter::Bracket, _) => {} + _ => panic!("expected brackets"), + } + *slice = &slice[2..]; +} + +fn assert_foo(slice: &mut &[TokenTree]) { + match slice[0].kind { + TokenNode::Term(ref name) => assert_eq!(name.as_str(), "fn"), + _ => panic!("expected fn"), + } + match slice[1].kind { + TokenNode::Term(ref name) => assert_eq!(name.as_str(), "foo"), + _ => panic!("expected foo"), + } + match slice[2].kind { + TokenNode::Group(Delimiter::Parenthesis, ref s) => assert!(s.is_empty()), + _ => panic!("expected parens"), + } + match slice[3].kind { + TokenNode::Group(Delimiter::Brace, _) => {} + _ => panic!("expected braces"), + } + *slice = &slice[4..]; +} + +fn fold_stream(input: TokenStream) -> TokenStream { + input.into_iter().map(fold_tree).collect() +} + +fn fold_tree(input: TokenTree) -> TokenTree { + TokenTree { + span: input.span, + kind: fold_node(input.kind), + } +} + +fn fold_node(input: TokenNode) -> TokenNode { + match input { + TokenNode::Group(a, b) => TokenNode::Group(a, fold_stream(b)), + TokenNode::Op(a, b) => TokenNode::Op(a, b), + TokenNode::Term(a) => TokenNode::Term(a), + TokenNode::Literal(a) => { + if a.to_string() != "\"foo\"" { + TokenNode::Literal(a) + } else { + TokenNode::Literal(Literal::integer(3)) + } + } + } +} -- cgit 1.4.1-3-g733a5 From 851c77088db304f0fba5318db39b3b31521aa274 Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Fri, 21 Jul 2017 19:29:43 -0400 Subject: default binding modes: add pat_binding_modes This PR kicks off the implementation of the [default binding modes RFC][1] by introducing the `pat_binding_modes` typeck table mentioned in the [mentoring instructions][2]. `pat_binding_modes` is populated in `librustc_typeck/check/_match.rs` and used wherever the HIR would be scraped prior to this PR. Unfortunately, one blemish, namely a two callers to `contains_explicit_ref_binding`, remains. This will likely have to be removed when the second part of [1], the `pat_adjustments` table, is tackled. Appropriate comments have been added. See #42640. [1]: https://github.com/rust-lang/rfcs/pull/2005 [2]: https://github.com/rust-lang/rust/issues/42640#issuecomment-313535089 --- src/librustc/hir/lowering.rs | 22 ++++++++++------- src/librustc/hir/mod.rs | 27 +++++++++++++++++---- src/librustc/hir/pat_util.rs | 40 ++++++++++++++++++------------- src/librustc/hir/print.rs | 12 ++++++---- src/librustc/ich/impls_hir.rs | 8 ++++--- src/librustc/ich/impls_ty.rs | 2 ++ src/librustc/middle/expr_use_visitor.rs | 28 ++++++++++++---------- src/librustc/middle/mem_categorization.rs | 34 +++++++++++++++----------- src/librustc/middle/region.rs | 26 +++++++++++++++++++- src/librustc/ty/binding.rs | 35 +++++++++++++++++++++++++++ src/librustc/ty/context.rs | 5 ++++ src/librustc/ty/mod.rs | 4 ++++ src/librustc_borrowck/borrowck/mod.rs | 11 +++++---- src/librustc_const_eval/check_match.rs | 26 ++++++++++++++------ src/librustc_const_eval/pattern.rs | 20 +++++++++------- src/librustc_lint/unused.rs | 8 +++++-- src/librustc_resolve/lib.rs | 3 ++- src/librustc_typeck/check/_match.rs | 25 +++++++++++++------ src/librustc_typeck/check/mod.rs | 4 +++- src/librustc_typeck/check/regionck.rs | 10 +++++--- src/librustc_typeck/check/writeback.rs | 9 +++++++ src/libsyntax/parse/mod.rs | 15 ++++++------ 22 files changed, 268 insertions(+), 106 deletions(-) create mode 100644 src/librustc/ty/binding.rs (limited to 'src/libsyntax/parse') diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 3ae3671b593..421a81c0d23 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2191,7 +2191,7 @@ impl<'a> LoweringContext<'a> { let next_ident = self.str_to_ident("__next"); let next_pat = self.pat_ident_binding_mode(e.span, next_ident, - hir::BindByValue(hir::MutMutable)); + hir::BindingAnnotation::Mutable); // `::std::option::Option::Some(val) => next = val` let pat_arm = { @@ -2215,8 +2215,9 @@ impl<'a> LoweringContext<'a> { }; // `mut iter` - let iter_pat = self.pat_ident_binding_mode(e.span, iter, - hir::BindByValue(hir::MutMutable)); + let iter_pat = self.pat_ident_binding_mode(e.span, + iter, + hir::BindingAnnotation::Mutable); // `match ::std::iter::Iterator::next(&mut iter) { ... }` let match_expr = { @@ -2503,10 +2504,13 @@ impl<'a> LoweringContext<'a> { } } - fn lower_binding_mode(&mut self, b: &BindingMode) -> hir::BindingMode { + fn lower_binding_mode(&mut self, b: &BindingMode) -> hir::BindingAnnotation { match *b { - BindingMode::ByRef(m) => hir::BindByRef(self.lower_mutability(m)), - BindingMode::ByValue(m) => hir::BindByValue(self.lower_mutability(m)), + BindingMode::ByValue(Mutability::Immutable) => + hir::BindingAnnotation::Unannotated, + BindingMode::ByRef(Mutability::Immutable) => hir::BindingAnnotation::Ref, + BindingMode::ByValue(Mutability::Mutable) => hir::BindingAnnotation::Mutable, + BindingMode::ByRef(Mutability::Mutable) => hir::BindingAnnotation::RefMut, } } @@ -2647,7 +2651,7 @@ impl<'a> LoweringContext<'a> { fn stmt_let(&mut self, sp: Span, mutbl: bool, ident: Name, ex: P) -> (hir::Stmt, NodeId) { let pat = if mutbl { - self.pat_ident_binding_mode(sp, ident, hir::BindByValue(hir::MutMutable)) + self.pat_ident_binding_mode(sp, ident, hir::BindingAnnotation::Mutable) } else { self.pat_ident(sp, ident) }; @@ -2703,10 +2707,10 @@ impl<'a> LoweringContext<'a> { } fn pat_ident(&mut self, span: Span, name: Name) -> P { - self.pat_ident_binding_mode(span, name, hir::BindByValue(hir::MutImmutable)) + self.pat_ident_binding_mode(span, name, hir::BindingAnnotation::Unannotated) } - fn pat_ident_binding_mode(&mut self, span: Span, name: Name, bm: hir::BindingMode) + fn pat_ident_binding_mode(&mut self, span: Span, name: Name, bm: hir::BindingAnnotation) -> P { let id = self.next_id(); let parent_def = self.parent_def.unwrap(); diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index a3a133daa09..7f1d1480d46 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -10,7 +10,6 @@ // The Rust HIR. -pub use self::BindingMode::*; pub use self::BinOp_::*; pub use self::BlockCheckMode::*; pub use self::CaptureClause::*; @@ -628,10 +627,28 @@ pub struct FieldPat { pub is_shorthand: bool, } +/// Explicit binding annotations given in the HIR for a binding. Note +/// that this is not the final binding *mode* that we infer after type +/// inference. #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)] -pub enum BindingMode { - BindByRef(Mutability), - BindByValue(Mutability), +pub enum BindingAnnotation { + /// No binding annotation given: this means that the final binding mode + /// will depend on whether we have skipped through a `&` reference + /// when matching. For example, the `x` in `Some(x)` will have binding + /// mode `None`; if you do `let Some(x) = &Some(22)`, it will + /// ultimately be inferred to be by-reference. + /// + /// Note that implicit reference skipping is not implemented yet (#42640). + Unannotated, + + /// Annotated with `mut x` -- could be either ref or not, similar to `None`. + Mutable, + + /// Annotated as `ref`, like `ref x` + Ref, + + /// Annotated as `ref mut x`. + RefMut, } #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)] @@ -647,7 +664,7 @@ pub enum PatKind { /// A fresh binding `ref mut binding @ OPT_SUBPATTERN`. /// The `DefId` is for the definition of the variable being bound. - Binding(BindingMode, DefId, Spanned, Option>), + Binding(BindingAnnotation, DefId, Spanned, Option>), /// A struct or struct variant pattern, e.g. `Variant {x, y, ..}`. /// The `bool` is `true` in the presence of a `..`. diff --git a/src/librustc/hir/pat_util.rs b/src/librustc/hir/pat_util.rs index 0190e74df69..144cb34ee35 100644 --- a/src/librustc/hir/pat_util.rs +++ b/src/librustc/hir/pat_util.rs @@ -87,7 +87,7 @@ impl hir::Pat { /// Call `f` on every "binding" in a pattern, e.g., on `a` in /// `match foo() { Some(a) => (), None => () }` pub fn each_binding(&self, mut f: F) - where F: FnMut(hir::BindingMode, ast::NodeId, Span, &Spanned), + where F: FnMut(hir::BindingAnnotation, ast::NodeId, Span, &Spanned), { self.walk(|p| { if let PatKind::Binding(binding_mode, _, ref pth, _) = p.node { @@ -130,12 +130,10 @@ impl hir::Pat { pub fn simple_name(&self) -> Option { match self.node { - PatKind::Binding(hir::BindByValue(..), _, ref path1, None) => { - Some(path1.node) - } - _ => { - None - } + PatKind::Binding(hir::BindingAnnotation::Unannotated, _, ref path1, None) | + PatKind::Binding(hir::BindingAnnotation::Mutable, _, ref path1, None) => + Some(path1.node), + _ => None, } } @@ -163,16 +161,22 @@ impl hir::Pat { } /// Checks if the pattern contains any `ref` or `ref mut` bindings, - /// and if yes whether its containing mutable ones or just immutables ones. - pub fn contains_ref_binding(&self) -> Option { + /// and if yes whether it contains mutable or just immutables ones. + /// + /// FIXME(tschottdorf): this is problematic as the HIR is being scraped, + /// but ref bindings may be implicit after #42640. + pub fn contains_explicit_ref_binding(&self) -> Option { let mut result = None; - self.each_binding(|mode, _, _, _| { - if let hir::BindingMode::BindByRef(m) = mode { - // Pick Mutable as maximum - match result { - None | Some(hir::MutImmutable) => result = Some(m), - _ => (), + self.each_binding(|annotation, _, _, _| { + match annotation { + hir::BindingAnnotation::Ref => { + match result { + None | Some(hir::MutImmutable) => result = Some(hir::MutImmutable), + _ => (), + } } + hir::BindingAnnotation::RefMut => result = Some(hir::MutMutable), + _ => (), } }); result @@ -182,9 +186,11 @@ impl hir::Pat { impl hir::Arm { /// Checks if the patterns for this arm contain any `ref` or `ref mut` /// bindings, and if yes whether its containing mutable ones or just immutables ones. - pub fn contains_ref_binding(&self) -> Option { + pub fn contains_explicit_ref_binding(&self) -> Option { + // FIXME(tschottdorf): contains_explicit_ref_binding() must be removed + // for #42640. self.pats.iter() - .filter_map(|pat| pat.contains_ref_binding()) + .filter_map(|pat| pat.contains_explicit_ref_binding()) .max_by_key(|m| match *m { hir::MutMutable => 1, hir::MutImmutable => 0, diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index beaf65b77d8..abfb00a24a1 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -1651,12 +1651,16 @@ impl<'a> State<'a> { PatKind::Wild => self.s.word("_")?, PatKind::Binding(binding_mode, _, ref path1, ref sub) => { match binding_mode { - hir::BindByRef(mutbl) => { + hir::BindingAnnotation::Ref => { self.word_nbsp("ref")?; - self.print_mutability(mutbl)?; + self.print_mutability(hir::MutImmutable)?; } - hir::BindByValue(hir::MutImmutable) => {} - hir::BindByValue(hir::MutMutable) => { + hir::BindingAnnotation::RefMut => { + self.word_nbsp("ref")?; + self.print_mutability(hir::MutMutable)?; + } + hir::BindingAnnotation::Unannotated => {} + hir::BindingAnnotation::Mutable => { self.word_nbsp("mut")?; } } diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index 7805029a67f..b344084f580 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -442,9 +442,11 @@ impl_stable_hash_for!(struct hir::FieldPat { is_shorthand }); -impl_stable_hash_for!(enum hir::BindingMode { - BindByRef(mutability), - BindByValue(mutability) +impl_stable_hash_for!(enum hir::BindingAnnotation { + Unannotated, + Mutable, + Ref, + RefMut }); impl_stable_hash_for!(enum hir::RangeEnd { diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 3e227872848..e03cbb45414 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -617,6 +617,7 @@ for ty::TypeckTables<'tcx> { ref node_types, ref node_substs, ref adjustments, + ref pat_binding_modes, ref upvar_capture_map, ref closure_tys, ref closure_kinds, @@ -637,6 +638,7 @@ for ty::TypeckTables<'tcx> { ich::hash_stable_nodemap(hcx, hasher, node_types); ich::hash_stable_nodemap(hcx, hasher, node_substs); ich::hash_stable_nodemap(hcx, hasher, adjustments); + ich::hash_stable_nodemap(hcx, hasher, pat_binding_modes); ich::hash_stable_hashmap(hcx, hasher, upvar_capture_map, |hcx, up_var_id| { let ty::UpvarId { var_id, diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 259bd4f0999..87e933e85e2 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -796,16 +796,19 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { debug!("determine_pat_move_mode cmt_discr={:?} pat={:?}", cmt_discr, pat); return_if_err!(self.mc.cat_pattern(cmt_discr, pat, |cmt_pat, pat| { - match pat.node { - PatKind::Binding(hir::BindByRef(..), ..) => - mode.lub(BorrowingMatch), - PatKind::Binding(hir::BindByValue(..), ..) => { - match copy_or_move(&self.mc, self.param_env, &cmt_pat, PatBindingMove) { - Copy => mode.lub(CopyingMatch), - Move(..) => mode.lub(MovingMatch), + if let PatKind::Binding(..) = pat.node { + let bm = *self.mc.tables.pat_binding_modes.get(&pat.id) + .expect("missing binding mode"); + match bm { + ty::BindByReference(..) => + mode.lub(BorrowingMatch), + ty::BindByValue(..) => { + match copy_or_move(&self.mc, self.param_env, &cmt_pat, PatBindingMove) { + Copy => mode.lub(CopyingMatch), + Move(..) => mode.lub(MovingMatch), + } } } - _ => {} } })); } @@ -818,8 +821,9 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { let ExprUseVisitor { ref mc, ref mut delegate, param_env } = *self; return_if_err!(mc.cat_pattern(cmt_discr.clone(), pat, |cmt_pat, pat| { - if let PatKind::Binding(bmode, def_id, ..) = pat.node { + if let PatKind::Binding(_, def_id, ..) = pat.node { debug!("binding cmt_pat={:?} pat={:?} match_mode={:?}", cmt_pat, pat, match_mode); + let bm = *mc.tables.pat_binding_modes.get(&pat.id).expect("missing binding mode"); // pat_ty: the type of the binding being produced. let pat_ty = return_if_err!(mc.node_ty(pat.id)); @@ -832,14 +836,14 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { } // It is also a borrow or copy/move of the value being matched. - match bmode { - hir::BindByRef(m) => { + match bm { + ty::BindByReference(m) => { if let ty::TyRef(r, _) = pat_ty.sty { let bk = ty::BorrowKind::from_mutbl(m); delegate.borrow(pat.id, pat.span, cmt_pat, r, bk, RefBinding); } } - hir::BindByValue(..) => { + ty::BindByValue(..) => { let mode = copy_or_move(mc, param_env, &cmt_pat, PatBindingMove); debug!("walk_pat binding consuming pat"); delegate.consume_pat(pat, cmt_pat, mode); diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 557d4b24f30..b4993aafc4c 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -330,11 +330,12 @@ impl MutabilityCategory { ret } - fn from_local(tcx: TyCtxt, id: ast::NodeId) -> MutabilityCategory { + fn from_local(tcx: TyCtxt, tables: &ty::TypeckTables, id: ast::NodeId) -> MutabilityCategory { let ret = match tcx.hir.get(id) { hir_map::NodeLocal(p) => match p.node { - PatKind::Binding(bind_mode, ..) => { - if bind_mode == hir::BindByValue(hir::MutMutable) { + PatKind::Binding(..) => { + let bm = *tables.pat_binding_modes.get(&p.id).expect("missing binding mode"); + if bm == ty::BindByValue(hir::MutMutable) { McDeclared } else { McImmutable @@ -475,16 +476,21 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> { // *being borrowed* is. But ideally we would put in a more // fundamental fix to this conflated use of the node id. let ret_ty = match pat.node { - PatKind::Binding(hir::BindByRef(_), ..) => { - // a bind-by-ref means that the base_ty will be the type of the ident itself, - // but what we want here is the type of the underlying value being borrowed. - // So peel off one-level, turning the &T into T. - match base_ty.builtin_deref(false, ty::NoPreference) { - Some(t) => t.ty, - None => { - debug!("By-ref binding of non-derefable type {:?}", base_ty); - return Err(()); + PatKind::Binding(..) => { + let bm = *self.tables.pat_binding_modes.get(&pat.id).expect("missing binding mode"); + if let ty::BindByReference(_) = bm { + // a bind-by-ref means that the base_ty will be the type of the ident itself, + // but what we want here is the type of the underlying value being borrowed. + // So peel off one-level, turning the &T into T. + match base_ty.builtin_deref(false, ty::NoPreference) { + Some(t) => t.ty, + None => { + debug!("By-ref binding of non-derefable type {:?}", base_ty); + return Err(()); + } } + } else { + base_ty } } _ => base_ty, @@ -659,7 +665,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> { id, span, cat: Categorization::Local(vid), - mutbl: MutabilityCategory::from_local(self.tcx, vid), + mutbl: MutabilityCategory::from_local(self.tcx, self.tables, vid), ty: expr_ty, note: NoteNone })) @@ -711,7 +717,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> { let var_ty = self.node_ty(var_id)?; // Mutability of original variable itself - let var_mutbl = MutabilityCategory::from_local(self.tcx, var_id); + let var_mutbl = MutabilityCategory::from_local(self.tcx, self.tables, var_id); // Construct the upvar. This represents access to the field // from the environment (perhaps we should eventually desugar diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index 39cb5d1b8c8..9133a5e777d 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -889,8 +889,32 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, /// | ( ..., P&, ... ) /// | box P& fn is_binding_pat(pat: &hir::Pat) -> bool { + // Note that the code below looks for *explicit* refs only, that is, it won't + // know about *implicit* refs as introduced in #42640. + // + // This is not a problem. For example, consider + // + // let (ref x, ref y) = (Foo { .. }, Bar { .. }); + // + // Due to the explicit refs on the left hand side, the below code would signal + // that the temporary value on the right hand side should live until the end of + // the enclosing block (as opposed to being dropped after the let is complete). + // + // To create an implicit ref, however, you must have a borrowed value on the RHS + // already, as in this example (which won't compile before #42640): + // + // let Foo { x, .. } = &Foo { x: ..., ... }; + // + // in place of + // + // let Foo { ref x, .. } = Foo { ... }; + // + // In the former case (the implicit ref version), the temporary is created by the + // & expression, and its lifetime would be extended to the end of the block (due + // to a different rule, not the below code). match pat.node { - PatKind::Binding(hir::BindByRef(_), ..) => true, + PatKind::Binding(hir::BindingAnnotation::Ref, ..) | + PatKind::Binding(hir::BindingAnnotation::RefMut, ..) => true, PatKind::Struct(_, ref field_pats, _) => { field_pats.iter().any(|fp| is_binding_pat(&fp.node.pat)) diff --git a/src/librustc/ty/binding.rs b/src/librustc/ty/binding.rs new file mode 100644 index 00000000000..3db61b76cc5 --- /dev/null +++ b/src/librustc/ty/binding.rs @@ -0,0 +1,35 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use hir::BindingAnnotation::*; +use hir::BindingAnnotation; +use hir::Mutability; + +#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)] +pub enum BindingMode { + BindByReference(Mutability), + BindByValue(Mutability), +} + +impl BindingMode { + pub fn convert(ba: BindingAnnotation) -> BindingMode { + match ba { + Unannotated => BindingMode::BindByValue(Mutability::MutImmutable), + Mutable => BindingMode::BindByValue(Mutability::MutMutable), + Ref => BindingMode::BindByReference(Mutability::MutImmutable), + RefMut => BindingMode::BindByReference(Mutability::MutMutable), + } + } +} + +impl_stable_hash_for!(enum self::BindingMode { + BindByReference(mutability), + BindByValue(mutability) +}); diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 45ddd4c0ff1..be3cd99426d 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -40,6 +40,7 @@ use ty::layout::{Layout, TargetDataLayout}; use ty::inhabitedness::DefIdForest; use ty::maps; use ty::steal::Steal; +use ty::BindingMode; use util::nodemap::{NodeMap, NodeSet, DefIdSet}; use util::nodemap::{FxHashMap, FxHashSet}; use rustc_data_structures::accumulate_vec::AccumulateVec; @@ -223,6 +224,9 @@ pub struct TypeckTables<'tcx> { pub adjustments: NodeMap>>, + // Stores the actual binding mode for all instances of hir::BindingAnnotation. + pub pat_binding_modes: NodeMap, + /// Borrows pub upvar_capture_map: ty::UpvarCaptureMap<'tcx>, @@ -274,6 +278,7 @@ impl<'tcx> TypeckTables<'tcx> { node_types: FxHashMap(), node_substs: NodeMap(), adjustments: NodeMap(), + pat_binding_modes: NodeMap(), upvar_capture_map: FxHashMap(), closure_tys: NodeMap(), closure_kinds: NodeMap(), diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 804f47b5283..914419ede36 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -74,6 +74,9 @@ pub use self::sty::InferTy::*; pub use self::sty::RegionKind::*; pub use self::sty::TypeVariants::*; +pub use self::binding::BindingMode; +pub use self::binding::BindingMode::*; + pub use self::context::{TyCtxt, GlobalArenas, tls}; pub use self::context::{Lift, TypeckTables}; @@ -84,6 +87,7 @@ pub use self::trait_def::TraitDef; pub use self::maps::queries; pub mod adjustment; +pub mod binding; pub mod cast; pub mod error; pub mod fast_reject; diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 1bfc5805bc8..0124a77349c 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -871,14 +871,15 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { } } - fn local_binding_mode(&self, node_id: ast::NodeId) -> hir::BindingMode { + fn local_binding_mode(&self, node_id: ast::NodeId) -> ty::BindingMode { let pat = match self.tcx.hir.get(node_id) { hir_map::Node::NodeLocal(pat) => pat, node => bug!("bad node for local: {:?}", node) }; match pat.node { - hir::PatKind::Binding(mode, ..) => mode, + hir::PatKind::Binding(..) => + *self.tables.pat_binding_modes.get(&pat.id).expect("missing binding mode"), _ => bug!("local is not a binding: {:?}", pat) } } @@ -913,7 +914,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { Some(ImmutabilityBlame::ClosureEnv(_)) => {} Some(ImmutabilityBlame::ImmLocal(node_id)) => { let let_span = self.tcx.hir.span(node_id); - if let hir::BindingMode::BindByValue(..) = self.local_binding_mode(node_id) { + if let ty::BindByValue(..) = self.local_binding_mode(node_id) { if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) { let (_, is_implicit_self) = self.local_ty(node_id); if is_implicit_self && snippet != "self" { @@ -930,7 +931,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { Some(ImmutabilityBlame::LocalDeref(node_id)) => { let let_span = self.tcx.hir.span(node_id); match self.local_binding_mode(node_id) { - hir::BindingMode::BindByRef(..) => { + ty::BindByReference(..) => { let snippet = self.tcx.sess.codemap().span_to_snippet(let_span); if let Ok(snippet) = snippet { db.span_label( @@ -940,7 +941,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { ); } } - hir::BindingMode::BindByValue(..) => { + ty::BindByValue(..) => { if let (Some(local_ty), is_implicit_self) = self.local_ty(node_id) { if let Some(msg) = self.suggest_mut_for_immutable(local_ty, is_implicit_self) { diff --git a/src/librustc_const_eval/check_match.rs b/src/librustc_const_eval/check_match.rs index 95c8613232e..060ff503d4e 100644 --- a/src/librustc_const_eval/check_match.rs +++ b/src/librustc_const_eval/check_match.rs @@ -268,7 +268,12 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> { fn check_for_bindings_named_the_same_as_variants(cx: &MatchVisitor, pat: &Pat) { pat.walk(|p| { - if let PatKind::Binding(hir::BindByValue(hir::MutImmutable), _, name, None) = p.node { + if let PatKind::Binding(_, _, name, None) = p.node { + let bm = *cx.tables.pat_binding_modes.get(&p.id).expect("missing binding mode"); + if bm != ty::BindByValue(hir::MutImmutable) { + // Nothing to check. + return true; + } let pat_ty = cx.tables.pat_ty(p); if let ty::TyAdt(edef, _) = pat_ty.sty { if edef.is_enum() && edef.variants.iter().any(|variant| { @@ -452,8 +457,9 @@ fn check_legality_of_move_bindings(cx: &MatchVisitor, pats: &[P]) { let mut by_ref_span = None; for pat in pats { - pat.each_binding(|bm, _, span, _path| { - if let hir::BindByRef(..) = bm { + pat.each_binding(|_, id, span, _path| { + let bm = *cx.tables.pat_binding_modes.get(&id).expect("missing binding mode"); + if let ty::BindByReference(..) = bm { by_ref_span = Some(span); } }) @@ -484,10 +490,16 @@ fn check_legality_of_move_bindings(cx: &MatchVisitor, for pat in pats { pat.walk(|p| { - if let PatKind::Binding(hir::BindByValue(..), _, _, ref sub) = p.node { - let pat_ty = cx.tables.node_id_to_type(p.id); - if pat_ty.moves_by_default(cx.tcx, cx.param_env, pat.span) { - check_move(p, sub.as_ref().map(|p| &**p)); + if let PatKind::Binding(_, _, _, ref sub) = p.node { + let bm = *cx.tables.pat_binding_modes.get(&p.id).expect("missing binding mode"); + match bm { + ty::BindByValue(..) => { + let pat_ty = cx.tables.node_id_to_type(p.id); + if pat_ty.moves_by_default(cx.tcx, cx.param_env, pat.span) { + check_move(p, sub.as_ref().map(|p| &**p)); + } + } + _ => {} } } true diff --git a/src/librustc_const_eval/pattern.rs b/src/librustc_const_eval/pattern.rs index ab919da8152..f37a112a596 100644 --- a/src/librustc_const_eval/pattern.rs +++ b/src/librustc_const_eval/pattern.rs @@ -374,27 +374,31 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { } } - PatKind::Binding(bm, def_id, ref ident, ref sub) => { + PatKind::Binding(_, def_id, ref ident, ref sub) => { let id = self.tcx.hir.as_local_node_id(def_id).unwrap(); let var_ty = self.tables.node_id_to_type(pat.id); let region = match var_ty.sty { ty::TyRef(r, _) => Some(r), _ => None, }; + let bm = *self.tables.pat_binding_modes.get(&pat.id) + .expect("missing binding mode"); let (mutability, mode) = match bm { - hir::BindByValue(hir::MutMutable) => + ty::BindByValue(hir::MutMutable) => (Mutability::Mut, BindingMode::ByValue), - hir::BindByValue(hir::MutImmutable) => + ty::BindByValue(hir::MutImmutable) => (Mutability::Not, BindingMode::ByValue), - hir::BindByRef(hir::MutMutable) => - (Mutability::Not, BindingMode::ByRef(region.unwrap(), BorrowKind::Mut)), - hir::BindByRef(hir::MutImmutable) => - (Mutability::Not, BindingMode::ByRef(region.unwrap(), BorrowKind::Shared)), + ty::BindByReference(hir::MutMutable) => + (Mutability::Not, BindingMode::ByRef( + region.unwrap(), BorrowKind::Mut)), + ty::BindByReference(hir::MutImmutable) => + (Mutability::Not, BindingMode::ByRef( + region.unwrap(), BorrowKind::Shared)), }; // A ref x pattern is the same node used for x, and as such it has // x's type, which is &T, where we want T (the type being matched). - if let hir::BindByRef(_) = bm { + if let ty::BindByReference(_) = bm { if let ty::TyRef(_, mt) = ty.sty { ty = mt.ty; } else { diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 473c0f3ffda..d7d0dc7cb35 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -44,9 +44,13 @@ impl UnusedMut { let mut mutables = FxHashMap(); for p in pats { - p.each_binding(|mode, id, _, path1| { + p.each_binding(|_, id, span, path1| { + let bm = match cx.tables.pat_binding_modes.get(&id) { + Some(&bm) => bm, + None => span_bug!(span, "missing binding mode"), + }; let name = path1.node; - if let hir::BindByValue(hir::MutMutable) = mode { + if let ty::BindByValue(hir::MutMutable) = bm { if !name.as_str().starts_with("_") { match mutables.entry(name) { Vacant(entry) => { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 88013b45a05..349a21af895 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2277,8 +2277,9 @@ impl<'a> Resolver<'a> { false, pat.span) .and_then(LexicalScopeBinding::item); let resolution = binding.map(NameBinding::def).and_then(|def| { + let ivmode = BindingMode::ByValue(Mutability::Immutable); let always_binding = !pat_src.is_refutable() || opt_pat.is_some() || - bmode != BindingMode::ByValue(Mutability::Immutable); + bmode != ivmode; match def { Def::StructCtor(_, CtorKind::Const) | Def::VariantCtor(_, CtorKind::Const) | diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 68726a7b1c4..01d2986a53c 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -113,10 +113,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.demand_eqtype(pat.span, expected, rhs_ty); common_type } - PatKind::Binding(bm, def_id, _, ref sub) => { + PatKind::Binding(ba, def_id, _, ref sub) => { + // Note the binding mode in the typeck tables. For now, what we store is always + // identical to what could be scraped from the HIR, but this will change with + // default binding modes (#42640). + let bm = ty::BindingMode::convert(ba); + self.inh.tables.borrow_mut().pat_binding_modes.insert(pat.id, bm); + let typ = self.local_ty(pat.span, pat.id); match bm { - hir::BindByRef(mutbl) => { + ty::BindByReference(mutbl) => { // if the binding is like // ref x | ref const x | ref mut x // then `x` is assigned a value of type `&M T` where M is the mutability @@ -131,7 +137,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.demand_eqtype(pat.span, region_ty, typ); } // otherwise the type of x is the expected type T - hir::BindByValue(_) => { + ty::BindByValue(_) => { // As above, `T <: typeof(x)` is required but we // use equality, see (*) below. self.demand_eqtype(pat.span, expected, typ); @@ -396,11 +402,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { match_src: hir::MatchSource) -> Ty<'tcx> { let tcx = self.tcx; - // Not entirely obvious: if matches may create ref bindings, we - // want to use the *precise* type of the discriminant, *not* some - // supertype, as the "discriminant type" (issue #23116). + // Not entirely obvious: if matches may create ref bindings, we want to + // use the *precise* type of the discriminant, *not* some supertype, as + // the "discriminant type" (issue #23116). + // + // FIXME(tschottdorf): don't call contains_explicit_ref_binding, which + // is problematic as the HIR is being scraped, but ref bindings may be + // implicit after #42640. We need to make sure that pat_adjustments + // (once introduced) is populated by the time we get here. let contains_ref_bindings = arms.iter() - .filter_map(|a| a.contains_ref_binding()) + .filter_map(|a| a.contains_explicit_ref_binding()) .max_by_key(|m| match *m { hir::MutMutable => 1, hir::MutImmutable => 0, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 7f69885047b..37fd0dd1586 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3999,7 +3999,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { local: &'gcx hir::Local, init: &'gcx hir::Expr) -> Ty<'tcx> { - let ref_bindings = local.pat.contains_ref_binding(); + // FIXME(tschottdorf): contains_explicit_ref_binding() must be removed + // for #42640. + let ref_bindings = local.pat.contains_explicit_ref_binding(); let local_ty = self.local_ty(init.span, local.id); if let Some(m) = ref_bindings { diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 82207428efc..9b7ecc194ca 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -1196,9 +1196,13 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { mc.cat_pattern(discr_cmt, root_pat, |sub_cmt, sub_pat| { match sub_pat.node { // `ref x` pattern - PatKind::Binding(hir::BindByRef(mutbl), ..) => { - self.link_region_from_node_type(sub_pat.span, sub_pat.id, - mutbl, sub_cmt); + PatKind::Binding(..) => { + let bm = *mc.tables.pat_binding_modes.get(&sub_pat.id) + .expect("missing binding mode"); + if let ty::BindByReference(mutbl) = bm { + self.link_region_from_node_type(sub_pat.span, sub_pat.id, + mutbl, sub_cmt); + } } _ => {} } diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 81e5dae5477..0a323efabec 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -178,6 +178,15 @@ impl<'cx, 'gcx, 'tcx> Visitor<'gcx> for WritebackCx<'cx, 'gcx, 'tcx> { } fn visit_pat(&mut self, p: &'gcx hir::Pat) { + match p.node { + hir::PatKind::Binding(..) => { + let bm = *self.fcx.tables.borrow().pat_binding_modes.get(&p.id) + .expect("missing binding mode"); + self.tables.pat_binding_modes.insert(p.id, bm); + } + _ => {} + }; + self.visit_node_id(p.span, p.id); intravisit::walk_pat(self, p); } diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 7b105a8fa14..893bada2670 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -867,13 +867,14 @@ mod tests { pat: P(ast::Pat { id: ast::DUMMY_NODE_ID, node: PatKind::Ident( - ast::BindingMode::ByValue(ast::Mutability::Immutable), - Spanned{ - span: sp(6,7), - node: Ident::from_str("b")}, - None - ), - span: sp(6,7) + ast::BindingMode::ByValue( + ast::Mutability::Immutable), + Spanned{ + span: sp(6,7), + node: Ident::from_str("b")}, + None + ), + span: sp(6,7) }), id: ast::DUMMY_NODE_ID }], -- cgit 1.4.1-3-g733a5