From 816c1b191cbea4f4949bc584c2324e81caa6e34b Mon Sep 17 00:00:00 2001 From: matthew Date: Thu, 22 Mar 2018 08:57:26 -0700 Subject: [PATCH 1/3] Check for known but incorrect attributes - Change nested_visit_map so it will recusively check functions - Add visit_stmt and visit_expr for impl Visitor for CheckAttrVisitor and check for incorrect inline and repr attributes on staements and expressions - Add regression test for isssue #43988 --- src/librustc/hir/check_attr.rs | 91 +++++++++++++++++++++++++--- src/test/compile-fail/issue-43988.rs | 36 +++++++++++ 2 files changed, 118 insertions(+), 9 deletions(-) create mode 100644 src/test/compile-fail/issue-43988.rs diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index 9b2647ad4db2b..6e1b7dc86a295 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -14,6 +14,7 @@ //! conflicts between multiple such attributes attached to the same //! item. +use syntax_pos::Span; use ty::TyCtxt; use hir; @@ -27,6 +28,8 @@ enum Target { Enum, Const, ForeignMod, + Expression, + Statement, Other, } @@ -62,7 +65,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { let mut has_wasm_import_module = false; for attr in &item.attrs { if attr.check_name("inline") { - self.check_inline(attr, item, target) + self.check_inline(attr, &item.span, target) } else if attr.check_name("wasm_import_module") { has_wasm_import_module = true; if attr.value_str().is_none() { @@ -99,13 +102,13 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { } /// Check if an `#[inline]` is applied to a function. - fn check_inline(&self, attr: &hir::Attribute, item: &hir::Item, target: Target) { + fn check_inline(&self, attr: &hir::Attribute, span: &Span, target: Target) { if target != Target::Fn { struct_span_err!(self.tcx.sess, attr.span, E0518, "attribute should be applied to function") - .span_label(item.span, "not a function") + .span_label(*span, "not a function") .emit(); } } @@ -196,10 +199,12 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { } _ => continue, }; - struct_span_err!(self.tcx.sess, hint.span, E0517, - "attribute should be applied to {}", allowed_targets) - .span_label(item.span, format!("not {} {}", article, allowed_targets)) - .emit(); + self.emit_repr_error( + hint.span, + item.span, + &format!("attribute should be applied to {}", allowed_targets), + &format!("not {} {}", article, allowed_targets), + ) } // Just point at all repr hints if there are any incompatibilities. @@ -221,17 +226,85 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { "conflicting representation hints"); } } + + fn emit_repr_error( + &self, + hint_span: Span, + label_span: Span, + hint_message: &str, + label_message: &str, + ) { + struct_span_err!(self.tcx.sess, hint_span, E0517, "{}", hint_message) + .span_label(label_span, label_message) + .emit(); + } + + fn check_stmt_attributes(&self, stmt: &hir::Stmt) { + // When checking statements ignore expressions, they will be checked later + if let hir::Stmt_::StmtDecl(_, _) = stmt.node { + for attr in stmt.node.attrs() { + if attr.check_name("inline") { + self.check_inline(attr, &stmt.span, Target::Statement); + } + if attr.check_name("repr") { + self.emit_repr_error( + attr.span, + stmt.span, + &format!("attribute should not be applied to statements"), + &format!("not a struct, enum or union"), + ); + } + } + } + } + + fn check_expr_attributes(&self, expr: &hir::Expr) { + use hir::Expr_::*; + match expr.node { + // Assignments, Calls and Structs were handled by Items and Statements + ExprCall(..) | + ExprAssign(..) | + ExprMethodCall(..) | + ExprStruct(..) => return, + _ => (), + } + + for attr in expr.attrs.iter() { + if attr.check_name("inline") { + self.check_inline(attr, &expr.span, Target::Expression); + } + if attr.check_name("repr") { + self.emit_repr_error( + attr.span, + expr.span, + &format!("attribute should not be applied to an expression"), + &format!("not a struct, enum or union"), + ); + } + } + } } impl<'a, 'tcx> Visitor<'tcx> for CheckAttrVisitor<'a, 'tcx> { fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { - NestedVisitorMap::None + NestedVisitorMap::OnlyBodies(&self.tcx.hir) } fn visit_item(&mut self, item: &'tcx hir::Item) { let target = Target::from_item(item); self.check_attributes(item, target); - intravisit::walk_item(self, item); + intravisit::walk_item(self, item) + } + + + fn visit_stmt(&mut self, stmt: &'tcx hir::Stmt) { + self.check_stmt_attributes(stmt); + intravisit::walk_stmt(self, stmt) + } + + fn visit_expr(&mut self, expr: &'tcx hir::Expr) { + self.check_expr_attributes(expr); + intravisit::walk_expr(self, expr) } } diff --git a/src/test/compile-fail/issue-43988.rs b/src/test/compile-fail/issue-43988.rs new file mode 100644 index 0000000000000..78237e31ba06e --- /dev/null +++ b/src/test/compile-fail/issue-43988.rs @@ -0,0 +1,36 @@ +// Copyright 2018 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 main() { + + #[inline] + let _a = 4; + //~^^ ERROR attribute should be applied to function + + + #[inline(XYZ)] + let _b = 4; + //~^^ ERROR attribute should be applied to function + + #[repr(nothing)] + let _x = 0; + //~^^ ERROR attribute should not be applied to statements + + + #[repr(something_not_real)] + loop { + () + }; + //~^^^^ ERROR attribute should not be applied to an expression + + #[repr] + let _y = "123"; + //~^^ ERROR attribute should not be applied to statements +} From 48825bcb23e752c0072a66450ae38d9865ee718e Mon Sep 17 00:00:00 2001 From: matthew Date: Mon, 26 Mar 2018 19:41:19 -0700 Subject: [PATCH 2/3] Remove an unnecessary/incorrect match in the expression check function --- src/librustc/hir/check_attr.rs | 14 ++------------ src/test/compile-fail/issue-43988.rs | 12 +++++++++--- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index 6e1b7dc86a295..ecf8960c237d3 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -250,7 +250,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { self.emit_repr_error( attr.span, stmt.span, - &format!("attribute should not be applied to statements"), + &format!("attribute should not be applied a statement"), &format!("not a struct, enum or union"), ); } @@ -259,16 +259,6 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { } fn check_expr_attributes(&self, expr: &hir::Expr) { - use hir::Expr_::*; - match expr.node { - // Assignments, Calls and Structs were handled by Items and Statements - ExprCall(..) | - ExprAssign(..) | - ExprMethodCall(..) | - ExprStruct(..) => return, - _ => (), - } - for attr in expr.attrs.iter() { if attr.check_name("inline") { self.check_inline(attr, &expr.span, Target::Expression); @@ -278,7 +268,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { attr.span, expr.span, &format!("attribute should not be applied to an expression"), - &format!("not a struct, enum or union"), + &format!("not defining a struct, enum or union"), ); } } diff --git a/src/test/compile-fail/issue-43988.rs b/src/test/compile-fail/issue-43988.rs index 78237e31ba06e..c03cd67fef990 100644 --- a/src/test/compile-fail/issue-43988.rs +++ b/src/test/compile-fail/issue-43988.rs @@ -21,8 +21,7 @@ fn main() { #[repr(nothing)] let _x = 0; - //~^^ ERROR attribute should not be applied to statements - + //~^^ ERROR attribute should not be applied a statement #[repr(something_not_real)] loop { @@ -32,5 +31,12 @@ fn main() { #[repr] let _y = "123"; - //~^^ ERROR attribute should not be applied to statements + //~^^ ERROR attribute should not be applied a statement + + + fn foo() {} + + #[inline(ABC)] + foo(); + //~^^ ERROR attribute should be applied to function } From 4957a40d13a78e0f9f1208cc0528663c49f24386 Mon Sep 17 00:00:00 2001 From: matthew Date: Tue, 27 Mar 2018 08:39:15 -0700 Subject: [PATCH 3/3] Add extra test for expressions and fix typo in message --- src/librustc/hir/check_attr.rs | 2 +- src/test/compile-fail/issue-43988.rs | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index ecf8960c237d3..316ed07ca05d9 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -250,7 +250,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { self.emit_repr_error( attr.span, stmt.span, - &format!("attribute should not be applied a statement"), + &format!("attribute should not be applied to a statement"), &format!("not a struct, enum or union"), ); } diff --git a/src/test/compile-fail/issue-43988.rs b/src/test/compile-fail/issue-43988.rs index c03cd67fef990..ff1fdaef416c8 100644 --- a/src/test/compile-fail/issue-43988.rs +++ b/src/test/compile-fail/issue-43988.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![feature(stmt_expr_attributes)] + fn main() { #[inline] @@ -21,7 +23,7 @@ fn main() { #[repr(nothing)] let _x = 0; - //~^^ ERROR attribute should not be applied a statement + //~^^ ERROR attribute should not be applied to a statement #[repr(something_not_real)] loop { @@ -31,7 +33,7 @@ fn main() { #[repr] let _y = "123"; - //~^^ ERROR attribute should not be applied a statement + //~^^ ERROR attribute should not be applied to a statement fn foo() {} @@ -39,4 +41,8 @@ fn main() { #[inline(ABC)] foo(); //~^^ ERROR attribute should be applied to function + + let _z = #[repr] 1; + //~^ ERROR attribute should not be applied to an expression + }