Skip to content

Commit

Permalink
Check for known but incorrect attributes
Browse files Browse the repository at this point in the history
- Change nested_visit_map so it will recusively check function

- Add visit_stmt and visit_expr for impl Visitor and check for incorrect
inline and repr attributes on staements and expressions

- Add regression test for isssue rust-lang#43988
  • Loading branch information
tejom committed Mar 22, 2018
1 parent 6bfa7d0 commit f52572e
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 9 deletions.
98 changes: 89 additions & 9 deletions src/librustc/hir/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,21 @@
//! conflicts between multiple such attributes attached to the same
//! item.

use syntax_pos;
use ty::TyCtxt;

use hir;
use hir::intravisit::{self, Visitor, NestedVisitorMap};


This comment has been minimized.

Copy link
@abonander

abonander Mar 22, 2018

Looks like a blank line got in here.

#[derive(Copy, Clone, PartialEq)]
enum Target {
Fn,
Struct,
Union,
Enum,
Expression,
Statement,
Other,
}

Expand Down Expand Up @@ -58,7 +62,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
for attr in &item.attrs {
if let Some(name) = attr.name() {
if name == "inline" {
self.check_inline(attr, item, target)
self.check_inline(attr, &item.span, target)
}
}
}
Expand All @@ -67,13 +71,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: &syntax_pos::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();
}
}
Expand Down Expand Up @@ -164,10 +168,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.
Expand All @@ -189,17 +195,91 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
"conflicting representation hints");
}
}

fn emit_repr_error(
&self,
hint_span: hir::Span,

This comment has been minimized.

Copy link
@abonander

abonander Mar 22, 2018

hir::Span is just an import of syntax_pos::Span so I would just import it as Span and use that everywhere.

label_span: hir::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() {
match attr.name() {
Some(name) if name == "inline" => {
self.check_inline(attr, &stmt.span, Target::Statement);
}
Some(name) if 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"),
);
}
_ => continue,
}
}
}
}

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() {
match attr.name() {
Some(name) if name == "inline" => {

This comment has been minimized.

Copy link
@abonander

abonander Mar 22, 2018

This is where .check_name() comes in handy.

self.check_inline(attr, &expr.span, Target::Expression);
}
Some(name) if 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"),
);
}
_ => continue,
}
}
}
}

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)
}
}

Expand Down
36 changes: 36 additions & 0 deletions src/test/compile-fail/issue-43988.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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

This comment has been minimized.

Copy link
@abonander

abonander Mar 22, 2018

Missing the tilde on all these: e.g, //~^^, //~^^^^



#[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
}

0 comments on commit f52572e

Please sign in to comment.