Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint #[inline(always)] closure in #[target_feature] functions #133408

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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]`
Expand Down
25 changes: 17 additions & 8 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,15 +584,24 @@ 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 {
// 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
.extend(tcx.codegen_fn_attrs(owner_id).target_features.iter().copied());
}
}
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -1093,6 +1093,12 @@ pub struct TargetFeatureDisableOrEnable<'a> {
pub missing_features: Option<MissingFeatures>,
}

#[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;
Expand Down
40 changes: 40 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -5177,3 +5178,42 @@ declare_lint! {
reference: "issue #116558 <https://github.com/rust-lang/rust/issues/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)]
/// #![feature(target_feature_11)]
///
/// #[target_feature(enable = "avx")]
/// fn example() {
/// let closure = {
/// #[inline(always)] move || {}
/// };
/// closure()
/// }
///
/// ```
///
/// {{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 <https://github.com/rust-lang/rust/issues/108655>",
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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

Loading