Rollup merge of #33926 - jseyfried:fix_derive_span, r=nrc

Fix the span of generated `#[derive_*]` attributes

Fixes #33571.
r? @nrc

3 files changed, 38 insertions, 60 deletions

diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs
index 9095230df63..d6adec84e84 100644
--- a/src/libsyntax_ext/deriving/generic/mod.rs
+++ b/src/libsyntax_ext/deriving/generic/mod.rs
@@ -201,7 +201,7 @@ use syntax::codemap::{self, respan, DUMMY_SP};
 use syntax::codemap::Span;
 use syntax::errors::Handler;
 use syntax::util::move_map::MoveMap;
-use syntax::parse::token::{intern, keywords, InternedString};
+use syntax::parse::token::{keywords, InternedString};
 use syntax::ptr::P;
 
 use self::ty::{LifetimeBounds, Path, Ptr, PtrTy, Self_, Ty};
@@ -1420,31 +1420,13 @@ impl<'a> MethodDef<'a> {
 
 // general helper methods.
 impl<'a> TraitDef<'a> {
-    fn set_expn_info(&self,
-                     cx: &mut ExtCtxt,
-                     mut to_set: Span) -> Span {
-        let trait_name = match self.path.path.last() {
-            None => cx.span_bug(self.span, "trait with empty path in generic `derive`"),
-            Some(name) => *name
-        };
-        to_set.expn_id = cx.codemap().record_expansion(codemap::ExpnInfo {
-            call_site: to_set,
-            callee: codemap::NameAndSpan {
-                format: codemap::MacroAttribute(intern(&format!("derive({})", trait_name))),
-                span: Some(self.span),
-                allow_internal_unstable: false,
-            }
-        });
-        to_set
-    }
-
     fn summarise_struct(&self,
                         cx: &mut ExtCtxt,
                         struct_def: &VariantData) -> StaticFields {
         let mut named_idents = Vec::new();
         let mut just_spans = Vec::new();
         for field in struct_def.fields(){
-            let sp = self.set_expn_info(cx, field.span);
+            let sp = Span { expn_id: self.span.expn_id, ..field.span };
             match field.ident {
                 Some(ident) => named_idents.push((ident, sp)),
                 _ => just_spans.push(sp),
@@ -1486,7 +1468,7 @@ impl<'a> TraitDef<'a> {
         let mut paths = Vec::new();
         let mut ident_exprs = Vec::new();
         for (i, struct_field) in struct_def.fields().iter().enumerate() {
-            let sp = self.set_expn_info(cx, struct_field.span);
+            let sp = Span { expn_id: self.span.expn_id, ..struct_field.span };
             let ident = cx.ident_of(&format!("{}_{}", prefix, i));
             paths.push(codemap::Spanned{span: sp, node: ident});
             let val = cx.expr_deref(sp, cx.expr_path(cx.path_ident(sp,ident)));
diff --git a/src/libsyntax_ext/deriving/mod.rs b/src/libsyntax_ext/deriving/mod.rs
index 91c272c59c4..6fb6dee94ed 100644
--- a/src/libsyntax_ext/deriving/mod.rs
+++ b/src/libsyntax_ext/deriving/mod.rs
@@ -16,7 +16,7 @@ use syntax::ext::base::{ExtCtxt, SyntaxEnv, Annotatable};
 use syntax::ext::base::{MultiDecorator, MultiItemDecorator, MultiModifier};
 use syntax::ext::build::AstBuilder;
 use syntax::feature_gate;
-use syntax::codemap::Span;
+use syntax::codemap::{self, Span};
 use syntax::parse::token::{intern, intern_and_get_ident};
 use syntax::ptr::P;
 
@@ -94,37 +94,7 @@ fn expand_derive(cx: &mut ExtCtxt,
             }
 
             let mut found_partial_eq = false;
-            let mut found_eq = false;
-
-            // This span is **very** sensitive and crucial to
-            // getting the stability behavior we want. What we are
-            // doing is marking the generated `#[derive_*]` with the
-            // span of the `#[deriving(...)]` attribute (the
-            // entire attribute, not just the `PartialEq` or `Eq`
-            // part), but with the current backtrace. The current
-            // backtrace will contain a topmost entry that IS this
-            // `#[deriving(...)]` attribute and with the
-            // "allow-unstable" flag set to true.
-            //
-            // Note that we do NOT use the span of the `Eq`
-            // text itself. You might think this is
-            // equivalent, because the `Eq` appears within the
-            // `#[deriving(Eq)]` attribute, and hence we would
-            // inherit the "allows unstable" from the
-            // backtrace.  But in fact this is not always the
-            // case. The actual source text that led to
-            // deriving can be `#[$attr]`, for example, where
-            // `$attr == deriving(Eq)`. In that case, the
-            // "#[derive_*]" would be considered to
-            // originate not from the deriving call but from
-            // text outside the deriving call, and hence would
-            // be forbidden from using unstable
-            // content.
-            //
-            // See tests src/run-pass/rfc1445 for
-            // examples. --nmatsakis
-            let span = Span { expn_id: cx.backtrace(), .. span };
-            assert!(cx.parse_sess.codemap().span_allows_unstable(span));
+            let mut eq_span = None;
 
             for titem in traits.iter().rev() {
                 let tname = match titem.node {
@@ -144,8 +114,19 @@ fn expand_derive(cx: &mut ExtCtxt,
                     continue;
                 }
 
+                let span = Span {
+                    expn_id: cx.codemap().record_expansion(codemap::ExpnInfo {
+                        call_site: titem.span,
+                        callee: codemap::NameAndSpan {
+                            format: codemap::MacroAttribute(intern(&format!("derive({})", tname))),
+                            span: Some(titem.span),
+                            allow_internal_unstable: true,
+                        },
+                    }), ..titem.span
+                };
+
                 if &tname[..] == "Eq" {
-                    found_eq = true;
+                    eq_span = Some(span);
                 } else if &tname[..] == "PartialEq" {
                     found_partial_eq = true;
                 }
@@ -157,12 +138,13 @@ fn expand_derive(cx: &mut ExtCtxt,
 
             // RFC #1445. `#[derive(PartialEq, Eq)]` adds a (trusted)
             // `#[structural_match]` attribute.
-            if found_partial_eq && found_eq {
-                debug!("inserting structural_match with span {:?}", span);
-                let structural_match = intern_and_get_ident("structural_match");
-                item.attrs.push(cx.attribute(span,
-                                             cx.meta_word(span,
-                                                          structural_match)));
+            if let Some(eq_span) = eq_span {
+                if found_partial_eq {
+                    let structural_match = intern_and_get_ident("structural_match");
+                    item.attrs.push(cx.attribute(eq_span,
+                                                 cx.meta_word(eq_span,
+                                                              structural_match)));
+                }
             }
 
             item
diff --git a/src/test/compile-fail/issue-33571.rs b/src/test/compile-fail/issue-33571.rs
new file mode 100644
index 00000000000..5dfc41c8f4a
--- /dev/null
+++ b/src/test/compile-fail/issue-33571.rs
@@ -0,0 +1,14 @@
+// Copyright 2016 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.
+
+#[derive(Clone,
+         Sync, //~ ERROR this unsafe trait should be implemented explicitly
+         Copy)]
+enum Foo {}