Rollup merge of #138842 - Noratrieb:inline-exported, r=me,saethlin

Emit `unused_attributes` for `#[inline]` on exported functions

I saw someone post a code sample that contained these two attributes, which immediately made me suspicious.
My suspicions were confirmed when I did a small test and checked the compiler source code to confirm that in these cases, `#[inline]` is indeed ignored (because you can't exactly `LocalCopy`an unmangled symbol since that would lead to duplicate symbols, and doing a mix of an unmangled `GloballyShared` and mangled `LocalCopy` instantiation is too complicated for our current instatiation mode logic, which I don't want to change right now).

So instead, emit the usual unused attribute lint with a message saying that the attribute is ignored in this position.

I think this is not 100% true, since I expect LLVM `inlinehint` to still be applied to such a function, but that's not why people use this attribute, they use it for the `LocalCopy` instantiation mode, where it doesn't work.

r? saethlin as the instantiation guy

Procedurally, I think this should be fine to merge without any lang involvement, as this only does a very minor extension to an existing lint.

Author: matthiaskrgr

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

@@ -172,6 +172,8 @@ impl CodegenFnAttrs {
     /// * `#[no_mangle]` is present
     /// * `#[export_name(...)]` is present
     /// * `#[linkage]` is present
+    ///
+    /// Keep this in sync with the logic for the unused_attributes for `#[inline]` lint.
     pub fn contains_extern_indicator(&self) -> bool {
         self.flags.contains(CodegenFnAttrFlags::NO_MANGLE)
             || self.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL)

compiler/rustc_middle/src/mir/mono.rs

@@ -150,6 +150,7 @@ impl<'tcx> MonoItem<'tcx> {
 
         // If the function is #[naked] or contains any other attribute that requires exactly-once
         // instantiation:
+        // We emit an unused_attributes lint for this case, which should be kept in sync if possible.
         let codegen_fn_attrs = tcx.codegen_fn_attrs(instance.def_id());
         if codegen_fn_attrs.contains_extern_indicator()
             || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED)

compiler/rustc_passes/messages.ftl

@@ -383,6 +383,10 @@ passes_inline_ignored_constants =
     .warn = {-passes_previously_accepted}
     .note = {-passes_see_issue(issue: "65833")}
 
+passes_inline_ignored_for_exported =
+    `#[inline]` is ignored on externally exported functions
+    .help = externally exported functions are functions with `#[no_mangle]`, `#[export_name]`, or `#[linkage]`
+
 passes_inline_ignored_function_prototype =
     `#[inline]` is ignored on function prototypes
 

compiler/rustc_passes/src/check_attr.rs

@@ -451,6 +451,23 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                 });
             }
         }
+
+        // `#[inline]` is ignored if the symbol must be codegened upstream because it's exported.
+        if let Some(did) = hir_id.as_owner()
+            && self.tcx.def_kind(did).has_codegen_attrs()
+            && !matches!(attr.meta_item_list().as_deref(), Some([item]) if item.has_name(sym::never))
+        {
+            let attrs = self.tcx.codegen_fn_attrs(did);
+            // Not checking naked as `#[inline]` is forbidden for naked functions anyways.
+            if attrs.contains_extern_indicator() {
+                self.tcx.emit_node_span_lint(
+                    UNUSED_ATTRIBUTES,
+                    hir_id,
+                    attr.span(),
+                    errors::InlineIgnoredForExported {},
+                );
+            }
+        }
     }
 
     /// Checks that `#[coverage(..)]` is applied to a function/closure/method,

compiler/rustc_passes/src/errors.rs

@@ -1441,6 +1441,11 @@ pub(crate) struct OnlyHasEffectOn {
     pub target_name: String,
 }
 
+#[derive(LintDiagnostic)]
+#[diag(passes_inline_ignored_for_exported)]
+#[help]
+pub(crate) struct InlineIgnoredForExported {}
+
 #[derive(Diagnostic)]
 #[diag(passes_object_lifetime_err)]
 pub(crate) struct ObjectLifetimeErr {

tests/ui/lint/inline-exported.rs

@@ -0,0 +1,30 @@
+//! Ensure the unused_attributes lint fires for externally exported functions with `#[inline]`,
+//! because `#[inline]` is ignored for such functions.
+
+#![crate_type = "lib"]
+
+#![feature(linkage)]
+#![feature(naked_functions)]
+#![deny(unused_attributes)]
+
+#[inline]
+//~^ ERROR: `#[inline]` is ignored on externally exported functions
+#[no_mangle]
+fn no_mangle() {}
+
+#[inline]
+//~^ ERROR: `#[inline]` is ignored on externally exported functions
+#[export_name = "export_name"]
+fn export_name() {}
+
+#[inline]
+//~^ ERROR: `#[inline]` is ignored on externally exported functions
+#[linkage = "external"]
+fn external_linkage() {}
+
+#[inline]
+fn normal() {}
+
+#[inline]
+#[linkage = "internal"] // not exported
+fn internal_linkage() {}

tests/ui/lint/inline-exported.stderr

@@ -0,0 +1,31 @@
+error: `#[inline]` is ignored on externally exported functions
+  --> $DIR/inline-exported.rs:10:1
+   |
+LL | #[inline]
+   | ^^^^^^^^^
+   |
+   = help: externally exported functions are functions with `#[no_mangle]`, `#[export_name]`, or `#[linkage]`
+note: the lint level is defined here
+  --> $DIR/inline-exported.rs:8:9
+   |
+LL | #![deny(unused_attributes)]
+   |         ^^^^^^^^^^^^^^^^^
+
+error: `#[inline]` is ignored on externally exported functions
+  --> $DIR/inline-exported.rs:15:1
+   |
+LL | #[inline]
+   | ^^^^^^^^^
+   |
+   = help: externally exported functions are functions with `#[no_mangle]`, `#[export_name]`, or `#[linkage]`
+
+error: `#[inline]` is ignored on externally exported functions
+  --> $DIR/inline-exported.rs:20:1
+   |
+LL | #[inline]
+   | ^^^^^^^^^
+   |
+   = help: externally exported functions are functions with `#[no_mangle]`, `#[export_name]`, or `#[linkage]`
+
+error: aborting due to 3 previous errors
+

tests/ui/target-feature/invalid-attribute.stderr

@@ -98,6 +98,12 @@ LL |
 LL | trait Baz {}
    | ------------ not a function definition
 
+error: cannot use `#[inline(always)]` with `#[target_feature]`
+  --> $DIR/invalid-attribute.rs:69:1
+   |
+LL | #[inline(always)]
+   | ^^^^^^^^^^^^^^^^^
+
 error: attribute should be applied to a function definition
   --> $DIR/invalid-attribute.rs:74:1
    |
@@ -163,12 +169,6 @@ error: malformed `target_feature` attribute input
 LL | #[target_feature(disable = "baz")]
    |                  ^^^^^^^^^^^^^^^ help: must be of the form: `enable = ".."`
 
-error: cannot use `#[inline(always)]` with `#[target_feature]`
-  --> $DIR/invalid-attribute.rs:69:1
-   |
-LL | #[inline(always)]
-   | ^^^^^^^^^^^^^^^^^
-
 error[E0046]: not all trait items implemented, missing: `foo`
   --> $DIR/invalid-attribute.rs:81:1
    |