From 119ebd4dc6089296770ee55b41691db564c93e5a Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sun, 24 Nov 2024 01:18:24 -0500 Subject: [PATCH 1/2] Lint #[inline(always)] closure in #[target_feature] functions --- compiler/rustc_codegen_ssa/messages.ftl | 5 +++ .../rustc_codegen_ssa/src/codegen_attrs.rs | 26 +++++++++----- compiler/rustc_codegen_ssa/src/errors.rs | 8 ++++- compiler/rustc_lint_defs/src/builtin.rs | 36 +++++++++++++++++++ .../issue-108655-inline-always-closure.rs | 33 ++++++++++++++++- .../issue-108655-inline-always-closure.stderr | 25 +++++++++++++ 6 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.stderr diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl index 62db3d5a98cdd..c5ee505f4a2ee 100644 --- a/compiler/rustc_codegen_ssa/messages.ftl +++ b/compiler/rustc_codegen_ssa/messages.ftl @@ -82,6 +82,11 @@ codegen_ssa_incorrect_cgu_reuse_type = *[other] {""} }`{$expected_reuse}` +codegen_ssa_inline_always_closure_in_target_feature_function = + cannot use `#[inline(always)]` with `#[target_feature]` + .note = `#[target_feature]` will not be applied to this closure and may result in unpredictable code generation + .help = consider using `#[inline]` instead + codegen_ssa_insufficient_vs_code_product = VS Code is a different product, and is not sufficient. codegen_ssa_invalid_link_ordinal_nargs = incorrect number of arguments to `#[link_ordinal]` diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 32e9422e005ab..31a960f0df7ea 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -584,15 +584,25 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { // its parent function, which effectively inherits the features anyway. Boxing this closure // would result in this closure being compiled without the inherited target features, but this // is probably a poor usage of `#[inline(always)]` and easily avoided by not using the attribute. - if tcx.features().target_feature_11() - && tcx.is_closure_like(did.to_def_id()) - && codegen_fn_attrs.inline != InlineAttr::Always - { + if tcx.features().target_feature_11() && tcx.is_closure_like(did.to_def_id()) { let owner_id = tcx.parent(did.to_def_id()); - if tcx.def_kind(owner_id).has_codegen_attrs() { - codegen_fn_attrs - .target_features - .extend(tcx.codegen_fn_attrs(owner_id).target_features.iter().copied()); + if tcx.def_kind(owner_id).has_codegen_attrs() + && !tcx.codegen_fn_attrs(owner_id).target_features.is_empty() + { + if codegen_fn_attrs.inline == InlineAttr::Always { + if let Some(inline_span) = inline_span { + tcx.emit_node_span_lint( + lint::builtin::INLINE_ALWAYS_CLOSURE_IN_TARGET_FEATURE_FUNCTION, + rustc_hir::CRATE_HIR_ID, + inline_span, + errors::InlineAlwaysClosureInTargetFeatureFunction, + ); + } + } else { + codegen_fn_attrs + .target_features + .extend(tcx.codegen_fn_attrs(owner_id).target_features.iter().copied()); + } } } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index f93cb52ea3ea2..00c039fc0e2c7 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -10,7 +10,7 @@ use rustc_errors::codes::*; use rustc_errors::{ Diag, DiagArgValue, DiagCtxtHandle, Diagnostic, EmissionGuarantee, IntoDiagArg, Level, }; -use rustc_macros::{Diagnostic, Subdiagnostic}; +use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic}; use rustc_middle::ty::Ty; use rustc_middle::ty::layout::LayoutError; use rustc_span::{Span, Symbol}; @@ -1093,6 +1093,12 @@ pub struct TargetFeatureDisableOrEnable<'a> { pub missing_features: Option, } +#[derive(LintDiagnostic)] +#[diag(codegen_ssa_inline_always_closure_in_target_feature_function)] +#[note] +#[help] +pub struct InlineAlwaysClosureInTargetFeatureFunction; + #[derive(Subdiagnostic)] #[help(codegen_ssa_missing_features)] pub struct MissingFeatures; diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 9036741e07877..bfb081b5e6800 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -52,6 +52,7 @@ declare_lint_pass! { ILL_FORMED_ATTRIBUTE_INPUT, INCOMPLETE_INCLUDE, INEFFECTIVE_UNSTABLE_TRAIT_IMPL, + INLINE_ALWAYS_CLOSURE_IN_TARGET_FEATURE_FUNCTION, INLINE_NO_SANITIZE, INVALID_DOC_ATTRIBUTES, INVALID_MACRO_EXPORT_ARGUMENTS, @@ -5177,3 +5178,38 @@ declare_lint! { reference: "issue #116558 ", }; } + +declare_lint! { + /// The `inline_always_closure_in_target_feature_function` lint detects using + /// the `#[inline(always)]` attribute on closures within functions marked with + /// the `#[target_feature(enable = "...")]` attribute. + /// + /// ### Example + /// + /// ```rust,edition2021,compile_fail + /// #![deny(inline_always_closure_in_target_feature_function)] + /// + /// #[target_feature(enable = "avx")] + /// fn example() { + /// let closure = #[inline(always)] || {}; + /// } + /// + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Rust previously did not apply a function's target features to closures + /// within it. + /// The `#[inline(always)]` attribute is incompatible with target features + /// due to the possibility of target feature mismatches. + /// The `#[inline]` attribute may be used instead. + pub INLINE_ALWAYS_CLOSURE_IN_TARGET_FEATURE_FUNCTION, + Warn, + "detect closures marked #[inline(always)] inside functions marked #[target_feature]", + @future_incompatible = FutureIncompatibleInfo { + reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps, + reference: "issue #108655 ", + }; +} diff --git a/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.rs b/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.rs index 6bd810b0956d8..f3badcf36b0c4 100644 --- a/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.rs +++ b/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.rs @@ -5,12 +5,43 @@ #![feature(target_feature_11)] +pub fn okay() { + ({ + #[inline(always)] + move || {} + })(); +} + #[target_feature(enable = "avx")] -pub unsafe fn test() { +pub unsafe fn also_okay() { + ({ + #[inline] + move || {} + })(); +} + +#[target_feature(enable = "avx")] +pub unsafe fn warn() { ({ #[inline(always)] + //~^ WARNING: cannot use `#[inline(always)]` with `#[target_feature]` [inline_always_closure_in_target_feature_function] + //~^^ WARNING: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! move || {} })(); } +#[target_feature(enable = "avx")] +pub unsafe fn also_warn() { + ({ + move || { + ({ + #[inline(always)] + //~^ WARNING: cannot use `#[inline(always)]` with `#[target_feature]` [inline_always_closure_in_target_feature_function] + //~^^ WARNING: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + move || {} + })(); + } + })(); +} + fn main() {} diff --git a/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.stderr b/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.stderr new file mode 100644 index 0000000000000..b750b4aebc041 --- /dev/null +++ b/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.stderr @@ -0,0 +1,25 @@ +warning: cannot use `#[inline(always)]` with `#[target_feature]` + --> $DIR/issue-108655-inline-always-closure.rs:26:9 + | +LL | #[inline(always)] + | ^^^^^^^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #108655 + = note: `#[target_feature]` will not be applied to this closure and may result in unpredictable code generation + = help: consider using `#[inline]` instead + = note: `#[warn(inline_always_closure_in_target_feature_function)]` on by default + +warning: cannot use `#[inline(always)]` with `#[target_feature]` + --> $DIR/issue-108655-inline-always-closure.rs:38:17 + | +LL | #[inline(always)] + | ^^^^^^^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #108655 + = note: `#[target_feature]` will not be applied to this closure and may result in unpredictable code generation + = help: consider using `#[inline]` instead + +warning: 2 warnings emitted + From e36740046bc5df64baad2aa0026566528b4811fc Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sun, 24 Nov 2024 12:33:25 -0500 Subject: [PATCH 2/2] Emit lint even if span for inline attribute isn't found. Fix lint docs. --- compiler/rustc_codegen_ssa/src/codegen_attrs.rs | 15 +++++++-------- compiler/rustc_lint_defs/src/builtin.rs | 6 +++++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 31a960f0df7ea..e34b40d807973 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -590,14 +590,13 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { && !tcx.codegen_fn_attrs(owner_id).target_features.is_empty() { if codegen_fn_attrs.inline == InlineAttr::Always { - if let Some(inline_span) = inline_span { - tcx.emit_node_span_lint( - lint::builtin::INLINE_ALWAYS_CLOSURE_IN_TARGET_FEATURE_FUNCTION, - rustc_hir::CRATE_HIR_ID, - inline_span, - errors::InlineAlwaysClosureInTargetFeatureFunction, - ); - } + // Do *not* inherit target features here, that could be unsound. + tcx.emit_node_span_lint( + lint::builtin::INLINE_ALWAYS_CLOSURE_IN_TARGET_FEATURE_FUNCTION, + rustc_hir::CRATE_HIR_ID, + inline_span.unwrap_or(tcx.def_span(did)), + errors::InlineAlwaysClosureInTargetFeatureFunction, + ); } else { codegen_fn_attrs .target_features diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index bfb081b5e6800..ebd9e671a9fff 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -5188,10 +5188,14 @@ declare_lint! { /// /// ```rust,edition2021,compile_fail /// #![deny(inline_always_closure_in_target_feature_function)] + /// #![feature(target_feature_11)] /// /// #[target_feature(enable = "avx")] /// fn example() { - /// let closure = #[inline(always)] || {}; + /// let closure = { + /// #[inline(always)] move || {} + /// }; + /// closure() /// } /// /// ```