Adjust check_no_mangle and check_export_name to warn/error on `#[…

…no_mangle]`/`#[export_name]` on trait methods
rust-lang · Aug 12, 2021 · db13848 · db13848
1 parent 0bb2ea6
commit db13848
Show file tree

Hide file tree

Showing 5 changed files with 254 additions and 196 deletions.
diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
@@ -962,6 +962,10 @@ impl CheckAttrVisitor<'tcx> {
         }
     }
 
+    fn is_impl_item(&self, hir_id: HirId) -> bool {
+        matches!(self.tcx.hir().get(hir_id), hir::Node::ImplItem(..))
+    }
+
     /// Checks if `#[export_name]` is applied to a function or static. Returns `true` if valid.
     fn check_export_name(
         &self,
@@ -971,7 +975,8 @@ impl CheckAttrVisitor<'tcx> {
         target: Target,
     ) -> bool {
         match target {
-            Target::Static | Target::Fn | Target::Method(..) => true,
+            Target::Static | Target::Fn => true,
+            Target::Method(..) if self.is_impl_item(hir_id) => true,
             // FIXME(#80564): We permit struct fields, match arms and macro defs to have an
             // `#[export_name]` attribute with just a lint, because we previously
             // erroneously allowed it and some crates used it accidentally, to to be compatible
@@ -985,9 +990,9 @@ impl CheckAttrVisitor<'tcx> {
                     .sess
                     .struct_span_err(
                         attr.span,
-                        "attribute should be applied to a function or static",
+                        "attribute should be applied to a free function, impl method or static",
                     )
-                    .span_label(*span, "not a function or static")
+                    .span_label(*span, "not a free function, impl method or static")
                     .emit();
                 false
             }
@@ -1169,7 +1174,8 @@ impl CheckAttrVisitor<'tcx> {
     /// Checks if `#[no_mangle]` is applied to a function or static.
     fn check_no_mangle(&self, hir_id: HirId, attr: &Attribute, span: &Span, target: Target) {
         match target {
-            Target::Static | Target::Fn | Target::Method(..) => {}
+            Target::Static | Target::Fn => {}
+            Target::Method(..) if self.is_impl_item(hir_id) => {}
             // FIXME(#80564): We permit struct fields, match arms and macro defs to have an
             // `#[no_mangle]` attribute with just a lint, because we previously
             // erroneously allowed it and some crates used it accidentally, to to be compatible
@@ -1181,14 +1187,16 @@ impl CheckAttrVisitor<'tcx> {
                 // FIXME: #[no_mangle] was previously allowed on non-functions/statics and some
                 // crates used this, so only emit a warning.
                 self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
-                    lint.build("attribute should be applied to a function or static")
-                        .warn(
-                            "this was previously accepted by the compiler but is \
-                             being phased out; it will become a hard error in \
-                             a future release!",
-                        )
-                        .span_label(*span, "not a function or static")
-                        .emit();
+                    lint.build(
+                        "attribute should be applied to a free function, impl method or static",
+                    )
+                    .warn(
+                        "this was previously accepted by the compiler but is \
+                         being phased out; it will become a hard error in \
+                         a future release!",
+                    )
+                    .span_label(*span, "not a free function, impl method or static")
+                    .emit();
                 });
             }
         }

diff --git a/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.rs b/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.rs
@@ -1,5 +1,5 @@
 //~ NOTE: not an `extern crate` item
-//~^ NOTE: not a function or static
+//~^ NOTE: not a free function, impl method or static
 //~^^ NOTE: not a function or closure
 // This is testing whether various builtin attributes signals an
 // error or warning when put in "weird" places.
@@ -25,7 +25,7 @@
 #![no_link]
 //~^ ERROR: attribute should be applied to an `extern crate` item
 #![export_name = "2200"]
-//~^ ERROR: attribute should be applied to a function or static
+//~^ ERROR: attribute should be applied to a free function, impl method or static
 #![inline]
 //~^ ERROR: attribute should be applied to function or closure
 #[inline]
@@ -83,27 +83,37 @@ mod no_link {
 }
 
 #[export_name = "2200"]
-//~^ ERROR attribute should be applied to a function or static
+//~^ ERROR attribute should be applied to a free function, impl method or static
 mod export_name {
-    //~^ NOTE not a function or static
+    //~^ NOTE not a free function, impl method or static
 
     mod inner { #![export_name="2200"] }
-    //~^ ERROR attribute should be applied to a function or static
-    //~| NOTE not a function or static
+    //~^ ERROR attribute should be applied to a free function, impl method or static
+    //~| NOTE not a free function, impl method or static
 
     #[export_name = "2200"] fn f() { }
 
     #[export_name = "2200"] struct S;
-    //~^ ERROR attribute should be applied to a function or static
-    //~| NOTE not a function or static
+    //~^ ERROR attribute should be applied to a free function, impl method or static
+    //~| NOTE not a free function, impl method or static
 
     #[export_name = "2200"] type T = S;
-    //~^ ERROR attribute should be applied to a function or static
-    //~| NOTE not a function or static
+    //~^ ERROR attribute should be applied to a free function, impl method or static
+    //~| NOTE not a free function, impl method or static
 
     #[export_name = "2200"] impl S { }
-    //~^ ERROR attribute should be applied to a function or static
-    //~| NOTE not a function or static
+    //~^ ERROR attribute should be applied to a free function, impl method or static
+    //~| NOTE not a free function, impl method or static
+
+    trait Tr {
+        #[export_name = "2200"] fn foo();
+        //~^ ERROR attribute should be applied to a free function, impl method or static
+        //~| NOTE not a free function, impl method or static
+
+        #[export_name = "2200"] fn bar() {}
+        //~^ ERROR attribute should be applied to a free function, impl method or static
+        //~| NOTE not a free function, impl method or static
+    }
 }
 
 #[start]

diff --git a/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.stderr b/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.stderr
@@ -17,31 +17,31 @@ LL |     #[inline = "2100"] fn f() { }
    = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571>
 
 error: `start` attribute can only be used on functions
-  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:109:1
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:119:1
    |
 LL | #[start]
    | ^^^^^^^^
 
 error: `start` attribute can only be used on functions
-  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:112:17
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:122:17
    |
 LL |     mod inner { #![start] }
    |                 ^^^^^^^^^
 
 error: `start` attribute can only be used on functions
-  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:117:5
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:127:5
    |
 LL |     #[start] struct S;
    |     ^^^^^^^^
 
 error: `start` attribute can only be used on functions
-  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:120:5
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:130:5
    |
 LL |     #[start] type T = S;
    |     ^^^^^^^^
 
 error: `start` attribute can only be used on functions
-  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:123:5
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:133:5
    |
 LL |     #[start] impl S { }
    |     ^^^^^^^^
@@ -76,7 +76,7 @@ LL | |
 LL | | }
    | |_- not an `extern crate` item
 
-error: attribute should be applied to a function or static
+error: attribute should be applied to a free function, impl method or static
   --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:85:1
    |
 LL |   #[export_name = "2200"]
@@ -87,17 +87,17 @@ LL | |
 LL | |
 LL | |     mod inner { #![export_name="2200"] }
 ...  |
-LL | |
+LL | |     }
 LL | | }
-   | |_- not a function or static
+   | |_- not a free function, impl method or static
 
 error: attribute should be applied to an `extern crate` item
   --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:25:1
    |
 LL | #![no_link]
    | ^^^^^^^^^^^
 
-error: attribute should be applied to a function or static
+error: attribute should be applied to a free function, impl method or static
   --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:27:1
    |
 LL | #![export_name = "2200"]
@@ -199,31 +199,43 @@ error: attribute should be applied to an `extern crate` item
 LL |     #[no_link] impl S { }
    |     ^^^^^^^^^^ ---------- not an `extern crate` item
 
-error: attribute should be applied to a function or static
+error: attribute should be applied to a free function, impl method or static
   --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:90:17
    |
 LL |     mod inner { #![export_name="2200"] }
-   |     ------------^^^^^^^^^^^^^^^^^^^^^^-- not a function or static
+   |     ------------^^^^^^^^^^^^^^^^^^^^^^-- not a free function, impl method or static
 
-error: attribute should be applied to a function or static
+error: attribute should be applied to a free function, impl method or static
   --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:96:5
    |
 LL |     #[export_name = "2200"] struct S;
-   |     ^^^^^^^^^^^^^^^^^^^^^^^ --------- not a function or static
+   |     ^^^^^^^^^^^^^^^^^^^^^^^ --------- not a free function, impl method or static
 
-error: attribute should be applied to a function or static
+error: attribute should be applied to a free function, impl method or static
   --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:100:5
    |
 LL |     #[export_name = "2200"] type T = S;
-   |     ^^^^^^^^^^^^^^^^^^^^^^^ ----------- not a function or static
+   |     ^^^^^^^^^^^^^^^^^^^^^^^ ----------- not a free function, impl method or static
 
-error: attribute should be applied to a function or static
+error: attribute should be applied to a free function, impl method or static
   --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:104:5
    |
 LL |     #[export_name = "2200"] impl S { }
-   |     ^^^^^^^^^^^^^^^^^^^^^^^ ---------- not a function or static
+   |     ^^^^^^^^^^^^^^^^^^^^^^^ ---------- not a free function, impl method or static
+
+error: attribute should be applied to a free function, impl method or static
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:109:9
+   |
+LL |         #[export_name = "2200"] fn foo();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^ --------- not a free function, impl method or static
+
+error: attribute should be applied to a free function, impl method or static
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:113:9
+   |
+LL |         #[export_name = "2200"] fn bar() {}
+   |         ^^^^^^^^^^^^^^^^^^^^^^^ ----------- not a free function, impl method or static
 
-error: aborting due to 32 previous errors
+error: aborting due to 34 previous errors
 
 Some errors have detailed explanations: E0518, E0658.
 For more information about an error, try `rustc --explain E0518`.
diff --git a/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs.rs b/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs.rs
@@ -295,31 +295,43 @@ mod automatically_derived {
 }
 
 #[no_mangle]
-//~^ WARN attribute should be applied to a function or static [unused_attributes]
+//~^ WARN attribute should be applied to a free function, impl method or static [unused_attributes]
 //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
 mod no_mangle {
-    //~^ NOTE not a function or static
+    //~^ NOTE not a free function, impl method or static
     mod inner { #![no_mangle] }
-    //~^ WARN attribute should be applied to a function or static [unused_attributes]
+    //~^ WARN attribute should be applied to a free function, impl method or static [unused_attributes]
     //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-    //~| NOTE not a function or static
+    //~| NOTE not a free function, impl method or static
 
     #[no_mangle] fn f() { }
 
     #[no_mangle] struct S;
-    //~^ WARN attribute should be applied to a function or static [unused_attributes]
+    //~^ WARN attribute should be applied to a free function, impl method or static [unused_attributes]
     //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-    //~| NOTE not a function or static
+    //~| NOTE not a free function, impl method or static
 
     #[no_mangle] type T = S;
-    //~^ WARN attribute should be applied to a function or static [unused_attributes]
+    //~^ WARN attribute should be applied to a free function, impl method or static [unused_attributes]
     //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-    //~| NOTE not a function or static
+    //~| NOTE not a free function, impl method or static
 
     #[no_mangle] impl S { }
-    //~^ WARN attribute should be applied to a function or static [unused_attributes]
+    //~^ WARN attribute should be applied to a free function, impl method or static [unused_attributes]
     //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-    //~| NOTE not a function or static
+    //~| NOTE not a free function, impl method or static
+
+    trait Tr {
+        #[no_mangle] fn foo();
+        //~^ WARN attribute should be applied to a free function, impl method or static [unused_attributes]
+        //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+        //~| NOTE not a free function, impl method or static
+
+        #[no_mangle] fn bar() {}
+        //~^ WARN attribute should be applied to a free function, impl method or static [unused_attributes]
+        //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+        //~| NOTE not a free function, impl method or static
+    }
 }
 
 #[should_panic]