Skip to content

Commit 3b390d3

Browse files
borsbors
committed
Auto merge of #50334 - pietroalbini:beta-backports, r=alexcrichton
[beta] Yet another round of backports * #50092: Warn on pointless `#[derive]` in more places * #50227: Fix ICE with erroneous `impl Trait` in a trait impl #50092 also needed some tweaks on the beta branch (see my own two commits). r? @alexcrichton
2 parents ceaeabf + 81c564e commit 3b390d3

File tree

9 files changed

+179
-18
lines changed

9 files changed

+179
-18
lines changed

‎src/librustc/hir/intravisit.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ pub fn walk_local<'v, V: Visitor<'v>>(visitor: &mut V, local: &'v Local) {
404404
// Intentionally visiting the expr first - the initialization expr
405405
// dominates the local's definition.
406406
walk_list!(visitor, visit_expr, &local.init);
407-
407+
walk_list!(visitor, visit_attribute, local.attrs.iter());
408408
visitor.visit_id(local.id);
409409
visitor.visit_pat(&local.pat);
410410
walk_list!(visitor, visit_ty, &local.ty);
@@ -730,6 +730,7 @@ pub fn walk_generic_param<'v, V: Visitor<'v>>(visitor: &mut V, param: &'v Generi
730730
visitor.visit_name(ty_param.span, ty_param.name);
731731
walk_list!(visitor, visit_ty_param_bound, &ty_param.bounds);
732732
walk_list!(visitor, visit_ty, &ty_param.default);
733+
walk_list!(visitor, visit_attribute, ty_param.attrs.iter());
733734
}
734735
}
735736
}

‎src/librustc_resolve/macros.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ impl<'a> base::Resolver for Resolver<'a> {
207207
}
208208

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

231+
if !allow_derive { return None }
232+
231233
// Check for legacy derives
232234
for i in 0..attrs.len() {
233235
let name = unwrap_or!(attrs[i].name(), continue);

‎src/librustc_typeck/check/compare_method.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -730,8 +730,7 @@ fn compare_synthetic_generics<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
730730
if impl_ty.synthetic != trait_ty.synthetic {
731731
let impl_node_id = tcx.hir.as_local_node_id(impl_ty.def_id).unwrap();
732732
let impl_span = tcx.hir.span(impl_node_id);
733-
let trait_node_id = tcx.hir.as_local_node_id(trait_ty.def_id).unwrap();
734-
let trait_span = tcx.hir.span(trait_node_id);
733+
let trait_span = tcx.def_span(trait_ty.def_id);
735734
let mut err = struct_span_err!(tcx.sess,
736735
impl_span,
737736
E0643,

‎src/libsyntax/ast.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ impl Stmt {
833833

834834
pub fn is_item(&self) -> bool {
835835
match self.node {
836-
StmtKind::Local(_) => true,
836+
StmtKind::Item(_) => true,
837837
_ => false,
838838
}
839839
}

‎src/libsyntax/ext/base.rs‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,20 @@ impl Annotatable {
106106
}
107107
}
108108

109+
pub fn expect_stmt(self) -> ast::Stmt {
110+
match self {
111+
Annotatable::Stmt(stmt) => stmt.into_inner(),
112+
_ => panic!("expected statement"),
113+
}
114+
}
115+
116+
pub fn expect_expr(self) -> P<ast::Expr> {
117+
match self {
118+
Annotatable::Expr(expr) => expr,
119+
_ => panic!("expected expression"),
120+
}
121+
}
122+
109123
pub fn derive_allowed(&self) -> bool {
110124
match *self {
111125
Annotatable::Item(ref item) => match item.node {
@@ -631,7 +645,9 @@ pub trait Resolver {
631645

632646
fn resolve_imports(&mut self);
633647
// Resolves attribute and derive legacy macros from `#![plugin(..)]`.
634-
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<Attribute>) -> Option<Attribute>;
648+
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<Attribute>, allow_derive: bool)
649+
-> Option<Attribute>;
650+
635651
fn resolve_invoc(&mut self, invoc: &mut Invocation, scope: Mark, force: bool)
636652
-> Result<Option<Lrc<SyntaxExtension>>, Determinacy>;
637653
fn resolve_macro(&mut self, scope: Mark, path: &ast::Path, kind: MacroKind, force: bool)
@@ -657,7 +673,8 @@ impl Resolver for DummyResolver {
657673
fn add_builtin(&mut self, _ident: ast::Ident, _ext: Lrc<SyntaxExtension>) {}
658674

659675
fn resolve_imports(&mut self) {}
660-
fn find_legacy_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>) -> Option<Attribute> { None }
676+
fn find_legacy_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>, _allow_derive: bool)
677+
-> Option<Attribute> { None }
661678
fn resolve_invoc(&mut self, _invoc: &mut Invocation, _scope: Mark, _force: bool)
662679
-> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
663680
Err(Determinacy::Determined)

‎src/libsyntax/ext/expand.rs‎

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -141,15 +141,22 @@ impl ExpansionKind {
141141
}
142142

143143
fn expect_from_annotatables<I: IntoIterator<Item = Annotatable>>(self, items: I) -> Expansion {
144-
let items = items.into_iter();
144+
let mut items = items.into_iter();
145145
match self {
146146
ExpansionKind::Items =>
147147
Expansion::Items(items.map(Annotatable::expect_item).collect()),
148148
ExpansionKind::ImplItems =>
149149
Expansion::ImplItems(items.map(Annotatable::expect_impl_item).collect()),
150150
ExpansionKind::TraitItems =>
151151
Expansion::TraitItems(items.map(Annotatable::expect_trait_item).collect()),
152-
_ => unreachable!(),
152+
ExpansionKind::Stmts => Expansion::Stmts(items.map(Annotatable::expect_stmt).collect()),
153+
ExpansionKind::Expr => Expansion::Expr(
154+
items.next().expect("expected exactly one expression").expect_expr()
155+
),
156+
ExpansionKind::OptExpr =>
157+
Expansion::OptExpr(items.next().map(Annotatable::expect_expr)),
158+
ExpansionKind::Pat | ExpansionKind::Ty =>
159+
panic!("patterns and types aren't annotatable"),
153160
}
154161
}
155162
}
@@ -867,14 +874,15 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
867874
self.collect(kind, InvocationKind::Attr { attr, traits, item })
868875
}
869876

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

876883
item = item.map_attrs(|mut attrs| {
877-
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs) {
884+
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
885+
true) {
878886
attr = Some(legacy_attr_invoc);
879887
return attrs;
880888
}
@@ -889,6 +897,28 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
889897
(attr, traits, item)
890898
}
891899

900+
/// Alternative of `classify_item()` that ignores `#[derive]` so invocations fallthrough
901+
/// to the unused-attributes lint (making it an error on statements and expressions
902+
/// is a breaking change)
903+
fn classify_nonitem<T: HasAttrs>(&mut self, mut item: T) -> (Option<ast::Attribute>, T) {
904+
let mut attr = None;
905+
906+
item = item.map_attrs(|mut attrs| {
907+
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
908+
false) {
909+
attr = Some(legacy_attr_invoc);
910+
return attrs;
911+
}
912+
913+
if self.cx.ecfg.proc_macro_enabled() {
914+
attr = find_attr_invoc(&mut attrs);
915+
}
916+
attrs
917+
});
918+
919+
(attr, item)
920+
}
921+
892922
fn configure<T: HasAttrs>(&mut self, node: T) -> Option<T> {
893923
self.cfg.configure(node)
894924
}
@@ -899,6 +929,13 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
899929
let features = self.cx.ecfg.features.unwrap();
900930
for attr in attrs.iter() {
901931
feature_gate::check_attribute(attr, self.cx.parse_sess, features);
932+
933+
// macros are expanded before any lint passes so this warning has to be hardcoded
934+
if attr.path == "derive" {
935+
self.cx.struct_span_warn(attr.span, "`#[derive]` does nothing on macro invocations")
936+
.note("this may become a hard error in a future release")
937+
.emit();
938+
}
902939
}
903940
}
904941

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

922-
let (attr, derives, expr) = self.classify_item(expr);
959+
// ignore derives so they remain unused
960+
let (attr, expr) = self.classify_nonitem(expr);
923961

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

929967
// ExpansionKind::Expr requires the macro to emit an expression
930-
return self.collect_attr(attr, derives, Annotatable::Expr(P(expr)), ExpansionKind::Expr)
968+
return self.collect_attr(attr, vec![], Annotatable::Expr(P(expr)), ExpansionKind::Expr)
931969
.make_expr();
932970
}
933971

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

946-
let (attr, derives, expr) = self.classify_item(expr);
984+
// ignore derives so they remain unused
985+
let (attr, expr) = self.classify_nonitem(expr);
947986

948-
if attr.is_some() || !derives.is_empty() {
987+
if attr.is_some() {
949988
attr.as_ref().map(|a| self.cfg.maybe_emit_expr_attr_err(a));
950989

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

9831022
// we'll expand attributes on expressions separately
9841023
if !stmt.is_expr() {
985-
let (attr, derives, stmt_) = self.classify_item(stmt);
1024+
let (attr, derives, stmt_) = if stmt.is_item() {
1025+
self.classify_item(stmt)
1026+
} else {
1027+
// ignore derives on non-item statements so it falls through
1028+
// to the unused-attributes lint
1029+
let (attr, stmt) = self.classify_nonitem(stmt);
1030+
(attr, vec![], stmt)
1031+
};
9861032

9871033
if attr.is_some() || !derives.is_empty() {
9881034
return self.collect_attr(attr, derives,

‎src/test/compile-fail/impl-trait/impl-generic-mismatch.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,15 @@ impl Bar for () {
2828
//~^ Error method `bar` has incompatible signature for trait
2929
}
3030

31+
// With non-local trait (#49841):
32+
33+
use std::hash::{Hash, Hasher};
34+
35+
struct X;
36+
37+
impl Hash for X {
38+
fn hash(&self, hasher: &mut impl Hasher) {}
39+
//~^ Error method `hash` has incompatible signature for trait
40+
}
41+
3142
fn main() {}

‎src/test/ui/issue-49934.rs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// must-compile-successfully
12+
13+
#![warn(unused_attributes)] //~ NOTE lint level defined here
14+
15+
fn foo() {
16+
match 0 {
17+
#[derive(Debug)] //~ WARN unused attribute
18+
_ => (),
19+
}
20+
}
21+
22+
fn main() {
23+
// fold_stmt (Item)
24+
#[allow(dead_code)]
25+
#[derive(Debug)] // should not warn
26+
struct Foo;
27+
28+
// fold_stmt (Mac)
29+
#[derive(Debug)]
30+
//~^ WARN `#[derive]` does nothing on macro invocations
31+
//~| NOTE this may become a hard error in a future release
32+
println!("Hello, world!");
33+
34+
// fold_stmt (Semi)
35+
#[derive(Debug)] //~ WARN unused attribute
36+
"Hello, world!";
37+
38+
// fold_stmt (Local)
39+
#[derive(Debug)] //~ WARN unused attribute
40+
let _ = "Hello, world!";
41+
42+
let _ = [
43+
// fold_opt_expr
44+
#[derive(Debug)] //~ WARN unused attribute
45+
"Hello, world!"
46+
];
47+
}

‎src/test/ui/issue-49934.stderr‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
warning: `#[derive]` does nothing on macro invocations
2+
--> $DIR/issue-49934.rs:29:5
3+
|
4+
LL | #[derive(Debug)]
5+
| ^^^^^^^^^^^^^^^^
6+
|
7+
= note: this may become a hard error in a future release
8+
9+
warning: unused attribute
10+
--> $DIR/issue-49934.rs:17:9
11+
|
12+
LL | #[derive(Debug)] //~ WARN unused attribute
13+
| ^^^^^^^^^^^^^^^^
14+
|
15+
note: lint level defined here
16+
--> $DIR/issue-49934.rs:13:9
17+
|
18+
LL | #![warn(unused_attributes)] //~ NOTE lint level defined here
19+
| ^^^^^^^^^^^^^^^^^
20+
21+
warning: unused attribute
22+
--> $DIR/issue-49934.rs:35:5
23+
|
24+
LL | #[derive(Debug)] //~ WARN unused attribute
25+
| ^^^^^^^^^^^^^^^^
26+
27+
warning: unused attribute
28+
--> $DIR/issue-49934.rs:39:5
29+
|
30+
LL | #[derive(Debug)] //~ WARN unused attribute
31+
| ^^^^^^^^^^^^^^^^
32+
33+
warning: unused attribute
34+
--> $DIR/issue-49934.rs:44:9
35+
|
36+
LL | #[derive(Debug)] //~ WARN unused attribute
37+
| ^^^^^^^^^^^^^^^^
38+

0 commit comments

Comments
 (0)