about summary refs log tree commit diff
path: root/compiler
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-07-28 08:57:16 +0200
committerGitHub <noreply@github.com>2024-07-28 08:57:16 +0200
commita13f40dae64647c524c8d2a64e9b30f97d391377 (patch)
treee6ba63a91ca38d64d0dec2817883056374a9877b /compiler
parent3148b35f6a6226548929c12da529ab0d1a4b47d5 (diff)
parentc7e688eccd60ab05a00dea56ac9d984397a2d02e (diff)
downloadrust-a13f40dae64647c524c8d2a64e9b30f97d391377.tar.gz
rust-a13f40dae64647c524c8d2a64e9b30f97d391377.zip
Rollup merge of #127853 - folkertdev:naked-function-error-messages, r=bjorn3
`#[naked]`: report incompatible attributes

tracking issue: https://github.com/rust-lang/rust/issues/90957

this is a re-implementation of https://github.com/rust-lang/rust/pull/93809 by ``@bstrie`` which was closed 2 years ago due to inactivity.

This PR takes some of the final comments into account, specifically providing a little more context in error messages, and using an allow list to determine which attributes are compatible with `#[naked]`.

Notable attributes that are incompatible with `#[naked]` are:

  * `#[inline]`
  * `#[track_caller]`
  * ~~`#[target_feature]`~~ (this is now allowed, see PR discussion)
  * `#[test]`, `#[ignore]`, `#[should_panic]`

These attributes just directly conflict with what `#[naked]` should do.

Naked functions are still important for systems programming, embedded, and operating systems, so I'd like to move them forward.
Diffstat (limited to 'compiler')
-rw-r--r--compiler/rustc_builtin_macros/messages.ftl5
-rw-r--r--compiler/rustc_builtin_macros/src/errors.rs10
-rw-r--r--compiler/rustc_builtin_macros/src/test.rs8
-rw-r--r--compiler/rustc_error_codes/src/error_codes/E0736.md18
-rw-r--r--compiler/rustc_error_codes/src/error_codes/E0739.md2
-rw-r--r--compiler/rustc_passes/messages.ftl11
-rw-r--r--compiler/rustc_passes/src/check_attr.rs73
-rw-r--r--compiler/rustc_passes/src/errors.rs25
-rw-r--r--compiler/rustc_passes/src/naked_functions.rs14
9 files changed, 118 insertions, 48 deletions
diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl
index 876f3c2b135..a30ab236213 100644
--- a/compiler/rustc_builtin_macros/messages.ftl
+++ b/compiler/rustc_builtin_macros/messages.ftl
@@ -220,6 +220,11 @@ builtin_macros_multiple_defaults = multiple declared defaults
     .note = only one variant can be default
     .suggestion = make `{$ident}` default
 
+builtin_macros_naked_functions_testing_attribute =
+    cannot use `#[naked]` with testing attributes
+    .label = function marked with testing attribute here
+    .naked_attribute = `#[naked]` is incompatible with testing attributes
+
 builtin_macros_no_default_variant = no default declared
     .help = make a unit variant default by placing `#[default]` above it
     .suggestion = make `{$ident}` default
diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs
index 2e6bdae14a8..f17819474ad 100644
--- a/compiler/rustc_builtin_macros/src/errors.rs
+++ b/compiler/rustc_builtin_macros/src/errors.rs
@@ -923,3 +923,13 @@ pub(crate) struct ExpectedItem<'a> {
     pub span: Span,
     pub token: &'a str,
 }
+
+#[derive(Diagnostic)]
+#[diag(builtin_macros_naked_functions_testing_attribute, code = E0736)]
+pub struct NakedFunctionTestingAttribute {
+    #[primary_span]
+    #[label(builtin_macros_naked_attribute)]
+    pub naked_span: Span,
+    #[label]
+    pub testing_span: Span,
+}
diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs
index c0310a2f4b0..bb00c8de1b8 100644
--- a/compiler/rustc_builtin_macros/src/test.rs
+++ b/compiler/rustc_builtin_macros/src/test.rs
@@ -133,6 +133,14 @@ pub(crate) fn expand_test_or_bench(
         };
     };
 
+    if let Some(attr) = attr::find_by_name(&item.attrs, sym::naked) {
+        cx.dcx().emit_err(errors::NakedFunctionTestingAttribute {
+            testing_span: attr_sp,
+            naked_span: attr.span,
+        });
+        return vec![Annotatable::Item(item)];
+    }
+
     // check_*_signature will report any errors in the type so compilation
     // will fail. We shouldn't try to expand in this case because the errors
     // would be spurious.
diff --git a/compiler/rustc_error_codes/src/error_codes/E0736.md b/compiler/rustc_error_codes/src/error_codes/E0736.md
index 0f3d41ba66d..cb7633b7068 100644
--- a/compiler/rustc_error_codes/src/error_codes/E0736.md
+++ b/compiler/rustc_error_codes/src/error_codes/E0736.md
@@ -1,14 +1,20 @@
-`#[track_caller]` and `#[naked]` cannot both be applied to the same function.
+Functions marked with the `#[naked]` attribute are restricted in what other
+attributes they may be marked with.
+
+Notable attributes that are incompatible with `#[naked]` are:
+
+* `#[inline]`
+* `#[track_caller]`
+* `#[test]`, `#[ignore]`, `#[should_panic]`
 
 Erroneous code example:
 
 ```compile_fail,E0736
+#[inline]
 #[naked]
-#[track_caller]
 fn foo() {}
 ```
 
-This is primarily due to ABI incompatibilities between the two attributes.
-See [RFC 2091] for details on this and other limitations.
-
-[RFC 2091]: https://github.com/rust-lang/rfcs/blob/master/text/2091-inline-semantic.md
+These incompatibilities are due to the fact that naked functions deliberately
+impose strict restrictions regarding the code that the compiler is
+allowed to produce for this function.
diff --git a/compiler/rustc_error_codes/src/error_codes/E0739.md b/compiler/rustc_error_codes/src/error_codes/E0739.md
index 8d9039bef93..406d3d52779 100644
--- a/compiler/rustc_error_codes/src/error_codes/E0739.md
+++ b/compiler/rustc_error_codes/src/error_codes/E0739.md
@@ -1,4 +1,4 @@
-`#[track_caller]` can not be applied on struct.
+`#[track_caller]` must  be applied to a function
 
 Erroneous code example:
 
diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl
index 1d93cbaddd6..bfe0d54e645 100644
--- a/compiler/rustc_passes/messages.ftl
+++ b/compiler/rustc_passes/messages.ftl
@@ -69,9 +69,6 @@ passes_break_non_loop =
     .suggestion = use `break` on its own without a value inside this `{$kind}` loop
     .break_expr_suggestion = alternatively, you might have meant to use the available loop label
 
-passes_cannot_inline_naked_function =
-    naked functions cannot be inlined
-
 passes_cannot_stabilize_deprecated =
     an API can't be stabilized after it is deprecated
     .label = invalid version
@@ -485,6 +482,11 @@ passes_naked_functions_asm_block =
 passes_naked_functions_asm_options =
     asm options unsupported in naked functions: {$unsupported_options}
 
+passes_naked_functions_incompatible_attribute =
+    attribute incompatible with `#[naked]`
+    .label = the `{$attr}` attribute is incompatible with `#[naked]`
+    .naked_attribute = function marked with `#[naked]` here
+
 passes_naked_functions_must_use_noreturn =
     asm in naked functions must use `noreturn` option
     .suggestion = consider specifying that the asm block is responsible for returning from the function
@@ -492,9 +494,6 @@ passes_naked_functions_must_use_noreturn =
 passes_naked_functions_operands =
     only `const` and `sym` operands are supported in naked functions
 
-passes_naked_tracked_caller =
-    cannot use `#[track_caller]` with `#[naked]`
-
 passes_no_link =
     attribute should be applied to an `extern crate` item
     .label = not an `extern crate` item
diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
index 9cbd989cc0e..e0cf65d3f98 100644
--- a/compiler/rustc_passes/src/check_attr.rs
+++ b/compiler/rustc_passes/src/check_attr.rs
@@ -155,7 +155,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                 [sym::rustc_std_internal_symbol] => {
                     self.check_rustc_std_internal_symbol(attr, span, target)
                 }
-                [sym::naked] => self.check_naked(hir_id, attr, span, target),
+                [sym::naked] => self.check_naked(hir_id, attr, span, target, attrs),
                 [sym::rustc_never_returns_null_ptr] => {
                     self.check_applied_to_fn_or_method(hir_id, attr, span, target)
                 }
@@ -410,12 +410,71 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
     }
 
     /// Checks if `#[naked]` is applied to a function definition.
-    fn check_naked(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) -> bool {
+    fn check_naked(
+        &self,
+        hir_id: HirId,
+        attr: &Attribute,
+        span: Span,
+        target: Target,
+        attrs: &[Attribute],
+    ) -> bool {
+        // many attributes don't make sense in combination with #[naked].
+        // Notable attributes that are incompatible with `#[naked]` are:
+        //
+        // * `#[inline]`
+        // * `#[track_caller]`
+        // * `#[test]`, `#[ignore]`, `#[should_panic]`
+        //
+        // NOTE: when making changes to this list, check that `error_codes/E0736.md` remains accurate
+        const ALLOW_LIST: &[rustc_span::Symbol] = &[
+            // conditional compilation
+            sym::cfg,
+            sym::cfg_attr,
+            // testing (allowed here so better errors can be generated in `rustc_builtin_macros::test`)
+            sym::test,
+            sym::ignore,
+            sym::should_panic,
+            sym::bench,
+            // diagnostics
+            sym::allow,
+            sym::warn,
+            sym::deny,
+            sym::forbid,
+            sym::deprecated,
+            sym::must_use,
+            // abi, linking and FFI
+            sym::export_name,
+            sym::link_section,
+            sym::linkage,
+            sym::no_mangle,
+            sym::naked,
+            sym::instruction_set,
+            // code generation
+            sym::cold,
+            sym::target_feature,
+            // documentation
+            sym::doc,
+        ];
+
         match target {
             Target::Fn
-            | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true,
+            | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => {
+                for other_attr in attrs {
+                    if !ALLOW_LIST.iter().any(|name| other_attr.has_name(*name)) {
+                        self.dcx().emit_err(errors::NakedFunctionIncompatibleAttribute {
+                            span: other_attr.span,
+                            naked_span: attr.span,
+                            attr: other_attr.name_or_empty(),
+                        });
+
+                        return false;
+                    }
+                }
+
+                true
+            }
             // FIXME(#80564): We permit struct fields, match arms and macro defs to have an
-            // `#[allow_internal_unstable]` attribute with just a lint, because we previously
+            // `#[naked]` attribute with just a lint, because we previously
             // erroneously allowed it and some crates used it accidentally, to be compatible
             // with crates depending on them, we can't throw an error here.
             Target::Field | Target::Arm | Target::MacroDef => {
@@ -488,7 +547,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
         }
     }
 
-    /// Checks if a `#[track_caller]` is applied to a non-naked function. Returns `true` if valid.
+    /// Checks if a `#[track_caller]` is applied to a function. Returns `true` if valid.
     fn check_track_caller(
         &self,
         hir_id: HirId,
@@ -498,10 +557,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
         target: Target,
     ) -> bool {
         match target {
-            _ if attrs.iter().any(|attr| attr.has_name(sym::naked)) => {
-                self.dcx().emit_err(errors::NakedTrackedCaller { attr_span });
-                false
-            }
             Target::Fn => {
                 // `#[track_caller]` is not valid on weak lang items because they are called via
                 // `extern` declarations and `#[track_caller]` would alter their ABI.
diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs
index 58d27d5b4bb..b195ba973ce 100644
--- a/compiler/rustc_passes/src/errors.rs
+++ b/compiler/rustc_passes/src/errors.rs
@@ -80,13 +80,6 @@ pub struct AttrShouldBeAppliedToFn {
 }
 
 #[derive(Diagnostic)]
-#[diag(passes_naked_tracked_caller, code = E0736)]
-pub struct NakedTrackedCaller {
-    #[primary_span]
-    pub attr_span: Span,
-}
-
-#[derive(Diagnostic)]
 #[diag(passes_should_be_applied_to_fn, code = E0739)]
 pub struct TrackedCallerWrongLocation {
     #[primary_span]
@@ -1124,13 +1117,6 @@ pub struct UnlabeledCfInWhileCondition<'a> {
     pub cf_type: &'a str,
 }
 
-#[derive(Diagnostic)]
-#[diag(passes_cannot_inline_naked_function)]
-pub struct CannotInlineNakedFunction {
-    #[primary_span]
-    pub span: Span,
-}
-
 #[derive(LintDiagnostic)]
 #[diag(passes_undefined_naked_function_abi)]
 pub struct UndefinedNakedFunctionAbi;
@@ -1197,6 +1183,17 @@ pub struct NakedFunctionsMustUseNoreturn {
 }
 
 #[derive(Diagnostic)]
+#[diag(passes_naked_functions_incompatible_attribute, code = E0736)]
+pub struct NakedFunctionIncompatibleAttribute {
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    #[label(passes_naked_attribute)]
+    pub naked_span: Span,
+    pub attr: Symbol,
+}
+
+#[derive(Diagnostic)]
 #[diag(passes_attr_only_in_functions)]
 pub struct AttrOnlyInFunctions {
     #[primary_span]
diff --git a/compiler/rustc_passes/src/naked_functions.rs b/compiler/rustc_passes/src/naked_functions.rs
index 4c5089b8676..b72ce239c4a 100644
--- a/compiler/rustc_passes/src/naked_functions.rs
+++ b/compiler/rustc_passes/src/naked_functions.rs
@@ -14,9 +14,8 @@ use rustc_span::Span;
 use rustc_target::spec::abi::Abi;
 
 use crate::errors::{
-    CannotInlineNakedFunction, NakedFunctionsAsmBlock, NakedFunctionsAsmOptions,
-    NakedFunctionsMustUseNoreturn, NakedFunctionsOperands, NoPatterns, ParamsNotAllowed,
-    UndefinedNakedFunctionAbi,
+    NakedFunctionsAsmBlock, NakedFunctionsAsmOptions, NakedFunctionsMustUseNoreturn,
+    NakedFunctionsOperands, NoPatterns, ParamsNotAllowed, UndefinedNakedFunctionAbi,
 };
 
 pub(crate) fn provide(providers: &mut Providers) {
@@ -53,15 +52,6 @@ fn check_mod_naked_functions(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
         check_no_patterns(tcx, body.params);
         check_no_parameters_use(tcx, body);
         check_asm(tcx, def_id, body);
-        check_inline(tcx, def_id);
-    }
-}
-
-/// Check that the function isn't inlined.
-fn check_inline(tcx: TyCtxt<'_>, def_id: LocalDefId) {
-    let attrs = tcx.get_attrs(def_id, sym::inline);
-    for attr in attrs {
-        tcx.dcx().emit_err(CannotInlineNakedFunction { span: attr.span });
     }
 }