Skip to content

[beta] Yet another round of backports #50334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ pub fn walk_local<'v, V: Visitor<'v>>(visitor: &mut V, local: &'v Local) {
// Intentionally visiting the expr first - the initialization expr
// dominates the local's definition.
walk_list!(visitor, visit_expr, &local.init);

walk_list!(visitor, visit_attribute, local.attrs.iter());
visitor.visit_id(local.id);
visitor.visit_pat(&local.pat);
walk_list!(visitor, visit_ty, &local.ty);
Expand Down Expand Up @@ -730,6 +730,7 @@ pub fn walk_generic_param<'v, V: Visitor<'v>>(visitor: &mut V, param: &'v Generi
visitor.visit_name(ty_param.span, ty_param.name);
walk_list!(visitor, visit_ty_param_bound, &ty_param.bounds);
walk_list!(visitor, visit_ty, &ty_param.default);
walk_list!(visitor, visit_attribute, ty_param.attrs.iter());
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl<'a> base::Resolver for Resolver<'a> {
}

// Resolves attribute and derive legacy macros from `#![plugin(..)]`.
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<ast::Attribute>)
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<ast::Attribute>, allow_derive: bool)
-> Option<ast::Attribute> {
for i in 0..attrs.len() {
let name = unwrap_or!(attrs[i].name(), continue);
Expand All @@ -228,6 +228,8 @@ impl<'a> base::Resolver for Resolver<'a> {
}
}

if !allow_derive { return None }

// Check for legacy derives
for i in 0..attrs.len() {
let name = unwrap_or!(attrs[i].name(), continue);
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_typeck/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,7 @@ fn compare_synthetic_generics<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
if impl_ty.synthetic != trait_ty.synthetic {
let impl_node_id = tcx.hir.as_local_node_id(impl_ty.def_id).unwrap();
let impl_span = tcx.hir.span(impl_node_id);
let trait_node_id = tcx.hir.as_local_node_id(trait_ty.def_id).unwrap();
let trait_span = tcx.hir.span(trait_node_id);
let trait_span = tcx.def_span(trait_ty.def_id);
let mut err = struct_span_err!(tcx.sess,
impl_span,
E0643,
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,7 @@ impl Stmt {

pub fn is_item(&self) -> bool {
match self.node {
StmtKind::Local(_) => true,
StmtKind::Item(_) => true,
_ => false,
}
}
Expand Down
21 changes: 19 additions & 2 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,20 @@ impl Annotatable {
}
}

pub fn expect_stmt(self) -> ast::Stmt {
match self {
Annotatable::Stmt(stmt) => stmt.into_inner(),
_ => panic!("expected statement"),
}
}

pub fn expect_expr(self) -> P<ast::Expr> {
match self {
Annotatable::Expr(expr) => expr,
_ => panic!("expected expression"),
}
}

pub fn derive_allowed(&self) -> bool {
match *self {
Annotatable::Item(ref item) => match item.node {
Expand Down Expand Up @@ -631,7 +645,9 @@ pub trait Resolver {

fn resolve_imports(&mut self);
// Resolves attribute and derive legacy macros from `#![plugin(..)]`.
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<Attribute>) -> Option<Attribute>;
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<Attribute>, allow_derive: bool)
-> Option<Attribute>;

fn resolve_invoc(&mut self, invoc: &mut Invocation, scope: Mark, force: bool)
-> Result<Option<Lrc<SyntaxExtension>>, Determinacy>;
fn resolve_macro(&mut self, scope: Mark, path: &ast::Path, kind: MacroKind, force: bool)
Expand All @@ -657,7 +673,8 @@ impl Resolver for DummyResolver {
fn add_builtin(&mut self, _ident: ast::Ident, _ext: Lrc<SyntaxExtension>) {}

fn resolve_imports(&mut self) {}
fn find_legacy_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>) -> Option<Attribute> { None }
fn find_legacy_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>, _allow_derive: bool)
-> Option<Attribute> { None }
fn resolve_invoc(&mut self, _invoc: &mut Invocation, _scope: Mark, _force: bool)
-> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
Err(Determinacy::Determined)
Expand Down
68 changes: 57 additions & 11 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,22 @@ impl ExpansionKind {
}

fn expect_from_annotatables<I: IntoIterator<Item = Annotatable>>(self, items: I) -> Expansion {
let items = items.into_iter();
let mut items = items.into_iter();
match self {
ExpansionKind::Items =>
Expansion::Items(items.map(Annotatable::expect_item).collect()),
ExpansionKind::ImplItems =>
Expansion::ImplItems(items.map(Annotatable::expect_impl_item).collect()),
ExpansionKind::TraitItems =>
Expansion::TraitItems(items.map(Annotatable::expect_trait_item).collect()),
_ => unreachable!(),
ExpansionKind::Stmts => Expansion::Stmts(items.map(Annotatable::expect_stmt).collect()),
ExpansionKind::Expr => Expansion::Expr(
items.next().expect("expected exactly one expression").expect_expr()
),
ExpansionKind::OptExpr =>
Expansion::OptExpr(items.next().map(Annotatable::expect_expr)),
ExpansionKind::Pat | ExpansionKind::Ty =>
panic!("patterns and types aren't annotatable"),
}
}
}
Expand Down Expand Up @@ -867,14 +874,15 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
self.collect(kind, InvocationKind::Attr { attr, traits, item })
}

// If `item` is an attr invocation, remove and return the macro attribute.
/// If `item` is an attr invocation, remove and return the macro attribute and derive traits.
fn classify_item<T>(&mut self, mut item: T) -> (Option<ast::Attribute>, Vec<Path>, T)
where T: HasAttrs,
{
let (mut attr, mut traits) = (None, Vec::new());

item = item.map_attrs(|mut attrs| {
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs) {
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
true) {
attr = Some(legacy_attr_invoc);
return attrs;
}
Expand All @@ -889,6 +897,28 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
(attr, traits, item)
}

/// Alternative of `classify_item()` that ignores `#[derive]` so invocations fallthrough
/// to the unused-attributes lint (making it an error on statements and expressions
/// is a breaking change)
fn classify_nonitem<T: HasAttrs>(&mut self, mut item: T) -> (Option<ast::Attribute>, T) {
let mut attr = None;

item = item.map_attrs(|mut attrs| {
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
false) {
attr = Some(legacy_attr_invoc);
return attrs;
}

if self.cx.ecfg.proc_macro_enabled() {
attr = find_attr_invoc(&mut attrs);
}
attrs
});

(attr, item)
}

fn configure<T: HasAttrs>(&mut self, node: T) -> Option<T> {
self.cfg.configure(node)
}
Expand All @@ -899,6 +929,13 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
let features = self.cx.ecfg.features.unwrap();
for attr in attrs.iter() {
feature_gate::check_attribute(attr, self.cx.parse_sess, features);

// macros are expanded before any lint passes so this warning has to be hardcoded
if attr.path == "derive" {
self.cx.struct_span_warn(attr.span, "`#[derive]` does nothing on macro invocations")
.note("this may become a hard error in a future release")
.emit();
}
}
}

Expand All @@ -919,15 +956,16 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
let mut expr = self.cfg.configure_expr(expr).into_inner();
expr.node = self.cfg.configure_expr_kind(expr.node);

let (attr, derives, expr) = self.classify_item(expr);
// ignore derives so they remain unused
let (attr, expr) = self.classify_nonitem(expr);

if attr.is_some() || !derives.is_empty() {
if attr.is_some() {
// collect the invoc regardless of whether or not attributes are permitted here
// expansion will eat the attribute so it won't error later
attr.as_ref().map(|a| self.cfg.maybe_emit_expr_attr_err(a));

// ExpansionKind::Expr requires the macro to emit an expression
return self.collect_attr(attr, derives, Annotatable::Expr(P(expr)), ExpansionKind::Expr)
return self.collect_attr(attr, vec![], Annotatable::Expr(P(expr)), ExpansionKind::Expr)
.make_expr();
}

Expand All @@ -943,12 +981,13 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
let mut expr = configure!(self, expr).into_inner();
expr.node = self.cfg.configure_expr_kind(expr.node);

let (attr, derives, expr) = self.classify_item(expr);
// ignore derives so they remain unused
let (attr, expr) = self.classify_nonitem(expr);

if attr.is_some() || !derives.is_empty() {
if attr.is_some() {
attr.as_ref().map(|a| self.cfg.maybe_emit_expr_attr_err(a));

return self.collect_attr(attr, derives, Annotatable::Expr(P(expr)),
return self.collect_attr(attr, vec![], Annotatable::Expr(P(expr)),
ExpansionKind::OptExpr)
.make_opt_expr();
}
Expand Down Expand Up @@ -982,7 +1021,14 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {

// we'll expand attributes on expressions separately
if !stmt.is_expr() {
let (attr, derives, stmt_) = self.classify_item(stmt);
let (attr, derives, stmt_) = if stmt.is_item() {
self.classify_item(stmt)
} else {
// ignore derives on non-item statements so it falls through
// to the unused-attributes lint
let (attr, stmt) = self.classify_nonitem(stmt);
(attr, vec![], stmt)
};

if attr.is_some() || !derives.is_empty() {
return self.collect_attr(attr, derives,
Expand Down
11 changes: 11 additions & 0 deletions src/test/compile-fail/impl-trait/impl-generic-mismatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,15 @@ impl Bar for () {
//~^ Error method `bar` has incompatible signature for trait
}

// With non-local trait (#49841):

use std::hash::{Hash, Hasher};

struct X;

impl Hash for X {
fn hash(&self, hasher: &mut impl Hasher) {}
//~^ Error method `hash` has incompatible signature for trait
}

fn main() {}
47 changes: 47 additions & 0 deletions src/test/ui/issue-49934.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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.

// must-compile-successfully

#![warn(unused_attributes)] //~ NOTE lint level defined here

fn foo() {
match 0 {
#[derive(Debug)] //~ WARN unused attribute
_ => (),
}
}

fn main() {
// fold_stmt (Item)
#[allow(dead_code)]
#[derive(Debug)] // should not warn
struct Foo;

// fold_stmt (Mac)
#[derive(Debug)]
//~^ WARN `#[derive]` does nothing on macro invocations
//~| NOTE this may become a hard error in a future release
println!("Hello, world!");

// fold_stmt (Semi)
#[derive(Debug)] //~ WARN unused attribute
"Hello, world!";

// fold_stmt (Local)
#[derive(Debug)] //~ WARN unused attribute
let _ = "Hello, world!";

let _ = [
// fold_opt_expr
#[derive(Debug)] //~ WARN unused attribute
"Hello, world!"
];
}
38 changes: 38 additions & 0 deletions src/test/ui/issue-49934.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
warning: `#[derive]` does nothing on macro invocations
--> $DIR/issue-49934.rs:29:5
|
LL | #[derive(Debug)]
| ^^^^^^^^^^^^^^^^
|
= note: this may become a hard error in a future release

warning: unused attribute
--> $DIR/issue-49934.rs:17:9
|
LL | #[derive(Debug)] //~ WARN unused attribute
| ^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/issue-49934.rs:13:9
|
LL | #![warn(unused_attributes)] //~ NOTE lint level defined here
| ^^^^^^^^^^^^^^^^^

warning: unused attribute
--> $DIR/issue-49934.rs:35:5
|
LL | #[derive(Debug)] //~ WARN unused attribute
| ^^^^^^^^^^^^^^^^

warning: unused attribute
--> $DIR/issue-49934.rs:39:5
|
LL | #[derive(Debug)] //~ WARN unused attribute
| ^^^^^^^^^^^^^^^^

warning: unused attribute
--> $DIR/issue-49934.rs:44:9
|
LL | #[derive(Debug)] //~ WARN unused attribute
| ^^^^^^^^^^^^^^^^