Skip to content

Commit

Permalink
only set non-ADT derive error once per attribute, not per trait
Browse files Browse the repository at this point in the history
A slight eccentricity of this change is that now non-ADT-derive errors prevent
derive-macro-not-found errors from surfacing (see changes to the
gating-of-derive compile-fail tests).

Resolves #43927.
  • Loading branch information
zackmdavis committed Sep 22, 2017
1 parent 17f56c5 commit 3517686
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 30 deletions.
18 changes: 18 additions & 0 deletions src/libsyntax/ext/expand.rs
Expand Up @@ -282,6 +282,24 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
let expansion = self.expand_invoc(invoc, ext);
self.collect_invocations(expansion, &[])
} else if let InvocationKind::Attr { attr: None, traits, item } = invoc.kind {
let derive_allowed = match item {
Annotatable::Item(ref item) => match item.node {
ast::ItemKind::Struct(..) |
ast::ItemKind::Enum(..) |
ast::ItemKind::Union(..) => true,
_ => false,
},
_ => false,
};
if !derive_allowed {
let span = item.attrs().iter()
.find(|attr| attr.check_name("derive"))
.expect("`derive` attribute should exist").span;
self.cx.span_err(span,
"`derive` may only be applied to structs, enums \
and unions");
}

let item = item
.map_attrs(|mut attrs| { attrs.retain(|a| a.path != "derive"); attrs });
let item_with_markers =
Expand Down
11 changes: 7 additions & 4 deletions src/libsyntax_ext/deriving/generic/mod.rs
Expand Up @@ -428,8 +428,9 @@ impl<'a> TraitDef<'a> {
}
}
_ => {
cx.span_err(mitem.span,
"`derive` may only be applied to structs, enums and unions");
// Non-ADT derive is an error, but it should have been
// set earlier; see
// libsyntax/ext/expand.rs:MacroExpander::expand()
return;
}
};
Expand All @@ -448,8 +449,10 @@ impl<'a> TraitDef<'a> {
push(Annotatable::Item(P(ast::Item { attrs: attrs, ..(*newitem).clone() })))
}
_ => {
cx.span_err(mitem.span,
"`derive` may only be applied to structs and enums");
// Non-Item derive is an error, but it should have been
// set earlier; see
// libsyntax/ext/expand.rs:MacroExpander::expand()
return;
}
}
}
Expand Down
Expand Up @@ -8,23 +8,9 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// `#![derive]` is interpreted (and raises errors) when it occurs at
// contexts other than ADT definitions. This test checks cases where
// the derive-macro does not exist.
// This test checks cases where the derive-macro does not exist.

#![derive(x3300)]
//~^ ERROR cannot find derive macro `x3300` in this scope

#[derive(x3300)]
//~^ ERROR cannot find derive macro `x3300` in this scope
mod derive {
mod inner { #![derive(x3300)] }
//~^ ERROR cannot find derive macro `x3300` in this scope

#[derive(x3300)]
//~^ ERROR cannot find derive macro `x3300` in this scope
fn derive() { }

#[derive(x3300)]
//~^ ERROR cannot find derive macro `x3300` in this scope
union U { f: i32 }
Expand All @@ -36,12 +22,4 @@ mod derive {
#[derive(x3300)]
//~^ ERROR cannot find derive macro `x3300` in this scope
struct S;

#[derive(x3300)]
//~^ ERROR cannot find derive macro `x3300` in this scope
type T = S;

#[derive(x3300)]
//~^ ERROR cannot find derive macro `x3300` in this scope
impl S { }
}
Expand Up @@ -8,9 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// `#![derive]` is interpreted (and raises errors) when it occurs at
// contexts other than ADT definitions. This test checks cases where
// the derive-macro exists.
// `#![derive]` raises errors when it occurs at contexts other than ADT
// definitions.

#![derive(Debug)]
//~^ ERROR `derive` may only be applied to structs, enums and unions
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/span/issue-43927-non-ADT-derive.rs
@@ -0,0 +1,16 @@
// 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 <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.

#![allow(dead_code)]

#![derive(Debug, PartialEq, Eq)] // should be an outer attribute!
struct DerivedOn;

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/span/issue-43927-non-ADT-derive.stderr
@@ -0,0 +1,8 @@
error: `derive` may only be applied to structs, enums and unions
--> $DIR/issue-43927-non-ADT-derive.rs:13:1
|
13 | #![derive(Debug, PartialEq, Eq)] // should be an outer attribute!
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

0 comments on commit 3517686

Please sign in to comment.