diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index 12f5f6ad79ab..a52a2369572e 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -101,6 +101,10 @@ impl FunctionData { flags.remove(FnFlags::HAS_UNSAFE_KW); } + if attrs.by_key(&sym::target_feature).exists() { + flags.insert(FnFlags::HAS_TARGET_FEATURE); + } + Arc::new(FunctionData { name: func.name.clone(), params: func @@ -155,6 +159,10 @@ impl FunctionData { pub fn is_varargs(&self) -> bool { self.flags.contains(FnFlags::IS_VARARGS) } + + pub fn has_target_feature(&self) -> bool { + self.flags.contains(FnFlags::HAS_TARGET_FEATURE) + } } fn parse_rustc_legacy_const_generics(tt: &crate::tt::TopSubtree) -> Box<[u32]> { diff --git a/crates/hir-def/src/item_tree.rs b/crates/hir-def/src/item_tree.rs index b5bf2feb82a2..79ee3344d256 100644 --- a/crates/hir-def/src/item_tree.rs +++ b/crates/hir-def/src/item_tree.rs @@ -937,7 +937,7 @@ pub struct Param { bitflags::bitflags! { #[derive(Debug, Clone, Copy, Eq, PartialEq, Default)] - pub(crate) struct FnFlags: u8 { + pub(crate) struct FnFlags: u16 { const HAS_SELF_PARAM = 1 << 0; const HAS_BODY = 1 << 1; const HAS_DEFAULT_KW = 1 << 2; @@ -946,6 +946,11 @@ bitflags::bitflags! { const HAS_UNSAFE_KW = 1 << 5; const IS_VARARGS = 1 << 6; const HAS_SAFE_KW = 1 << 7; + /// The `#[target_feature]` attribute is necessary to check safety (with RFC 2396), + /// but keeping it for all functions will consume a lot of memory when there are + /// only very few functions with it. So we only encode its existence here, and lookup + /// it if needed. + const HAS_TARGET_FEATURE = 1 << 8; } } diff --git a/crates/hir-ty/src/diagnostics/unsafe_check.rs b/crates/hir-ty/src/diagnostics/unsafe_check.rs index 6bba83fac988..5b0041d47271 100644 --- a/crates/hir-ty/src/diagnostics/unsafe_check.rs +++ b/crates/hir-ty/src/diagnostics/unsafe_check.rs @@ -14,7 +14,8 @@ use hir_def::{ }; use crate::{ - db::HirDatabase, utils::is_fn_unsafe_to_call, InferenceResult, Interner, TyExt, TyKind, + db::HirDatabase, utils::is_fn_unsafe_to_call, InferenceResult, Interner, TargetFeatures, TyExt, + TyKind, }; /// Returns `(unsafe_exprs, fn_is_unsafe)`. @@ -96,6 +97,7 @@ struct UnsafeVisitor<'a> { inside_assignment: bool, inside_union_destructure: bool, unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason), + def_target_features: TargetFeatures, } impl<'a> UnsafeVisitor<'a> { @@ -107,6 +109,10 @@ impl<'a> UnsafeVisitor<'a> { unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason), ) -> Self { let resolver = def.resolver(db.upcast()); + let def_target_features = match def { + DefWithBodyId::FunctionId(func) => TargetFeatures::from_attrs(&db.attrs(func.into())), + _ => TargetFeatures::default(), + }; Self { db, infer, @@ -117,6 +123,7 @@ impl<'a> UnsafeVisitor<'a> { inside_assignment: false, inside_union_destructure: false, unsafe_expr_cb, + def_target_features, } } @@ -181,7 +188,7 @@ impl<'a> UnsafeVisitor<'a> { match expr { &Expr::Call { callee, .. } => { if let Some(func) = self.infer[callee].as_fn_def(self.db) { - if is_fn_unsafe_to_call(self.db, func) { + if is_fn_unsafe_to_call(self.db, func, &self.def_target_features) { self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall); } } @@ -212,7 +219,7 @@ impl<'a> UnsafeVisitor<'a> { if self .infer .method_resolution(current) - .map(|(func, _)| is_fn_unsafe_to_call(self.db, func)) + .map(|(func, _)| is_fn_unsafe_to_call(self.db, func, &self.def_target_features)) .unwrap_or(false) { self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall); diff --git a/crates/hir-ty/src/lib.rs b/crates/hir-ty/src/lib.rs index 3c18ea928165..37de501e9878 100644 --- a/crates/hir-ty/src/lib.rs +++ b/crates/hir-ty/src/lib.rs @@ -100,7 +100,7 @@ pub use mapping::{ }; pub use method_resolution::check_orphan_rules; pub use traits::TraitEnvironment; -pub use utils::{all_super_traits, direct_super_traits, is_fn_unsafe_to_call}; +pub use utils::{all_super_traits, direct_super_traits, is_fn_unsafe_to_call, TargetFeatures}; pub use variance::Variance; pub use chalk_ir::{ diff --git a/crates/hir-ty/src/utils.rs b/crates/hir-ty/src/utils.rs index bf7892f69bd3..ffd0b5b5e21a 100644 --- a/crates/hir-ty/src/utils.rs +++ b/crates/hir-ty/src/utils.rs @@ -9,16 +9,18 @@ use chalk_ir::{ DebruijnIndex, }; use hir_def::{ + attr::Attrs, db::DefDatabase, generics::{WherePredicate, WherePredicateTypeTarget}, lang_item::LangItem, resolver::{HasResolver, TypeNs}, + tt, type_ref::{TraitBoundModifier, TypeRef}, EnumId, EnumVariantId, FunctionId, Lookup, OpaqueInternableThing, TraitId, TypeAliasId, TypeOrConstParamId, }; use hir_expand::name::Name; -use intern::sym; +use intern::{sym, Symbol}; use rustc_abi::TargetDataLayout; use rustc_hash::FxHashSet; use smallvec::{smallvec, SmallVec}; @@ -264,12 +266,50 @@ impl<'a> ClosureSubst<'a> { } } -pub fn is_fn_unsafe_to_call(db: &dyn HirDatabase, func: FunctionId) -> bool { +#[derive(Debug, Default)] +pub struct TargetFeatures { + enabled: FxHashSet, +} + +impl TargetFeatures { + pub fn from_attrs(attrs: &Attrs) -> Self { + let enabled = attrs + .by_key(&sym::target_feature) + .tt_values() + .filter_map(|tt| { + match tt.token_trees().flat_tokens() { + [ + tt::TokenTree::Leaf(tt::Leaf::Ident(enable_ident)), + tt::TokenTree::Leaf(tt::Leaf::Punct(tt::Punct { char: '=', .. })), + tt::TokenTree::Leaf(tt::Leaf::Literal(tt::Literal { kind: tt::LitKind::Str, symbol: features, .. })), + ] if enable_ident.sym == sym::enable => Some(features), + _ => None, + } + }) + .flat_map(|features| features.as_str().split(',').map(Symbol::intern)) + .collect(); + Self { enabled } + } +} + +pub fn is_fn_unsafe_to_call( + db: &dyn HirDatabase, + func: FunctionId, + caller_target_features: &TargetFeatures, +) -> bool { let data = db.function_data(func); if data.is_unsafe() { return true; } + if data.has_target_feature() { + // RFC 2396 . + let callee_target_features = TargetFeatures::from_attrs(&db.attrs(func.into())); + if !caller_target_features.enabled.is_superset(&callee_target_features.enabled) { + return true; + } + } + let loc = func.lookup(db.upcast()); match loc.container { hir_def::ItemContainerId::ExternBlockId(block) => { diff --git a/crates/hir/src/display.rs b/crates/hir/src/display.rs index b29c91694d37..6f4049705585 100644 --- a/crates/hir/src/display.rs +++ b/crates/hir/src/display.rs @@ -80,7 +80,9 @@ impl HirDisplay for Function { if data.is_async() { f.write_str("async ")?; } - if self.is_unsafe_to_call(db) { + // FIXME: This will show `unsafe` for functions that are `#[target_feature]` but not unsafe + // (they are conditionally unsafe to call). We probably should show something else. + if self.is_unsafe_to_call(db, None) { f.write_str("unsafe ")?; } if let Some(abi) = &data.abi { diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 0cbc75726bf3..407b1b94eda1 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -2361,8 +2361,11 @@ impl Function { db.attrs(self.id.into()).is_unstable() } - pub fn is_unsafe_to_call(self, db: &dyn HirDatabase) -> bool { - hir_ty::is_fn_unsafe_to_call(db, self.id) + pub fn is_unsafe_to_call(self, db: &dyn HirDatabase, caller: Option) -> bool { + let target_features = caller + .map(|caller| hir_ty::TargetFeatures::from_attrs(&db.attrs(caller.id.into()))) + .unwrap_or_default(); + hir_ty::is_fn_unsafe_to_call(db, self.id, &target_features) } /// Whether this function declaration has a definition. diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 09470bed9cfb..882a27182f01 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -2040,6 +2040,13 @@ impl SemanticsScope<'_> { Crate { id: self.resolver.krate() } } + pub fn containing_function(&self) -> Option { + self.resolver.body_owner().and_then(|owner| match owner { + DefWithBodyId::FunctionId(id) => Some(id.into()), + _ => None, + }) + } + pub(crate) fn resolver(&self) -> &Resolver { &self.resolver } diff --git a/crates/hir/src/term_search/tactics.rs b/crates/hir/src/term_search/tactics.rs index 1b0e6f8bd5b0..1ff9b6dec9c8 100644 --- a/crates/hir/src/term_search/tactics.rs +++ b/crates/hir/src/term_search/tactics.rs @@ -365,7 +365,7 @@ pub(super) fn free_function<'a, DB: HirDatabase>( let ret_ty = it.ret_type_with_args(db, generics.iter().cloned()); // Filter out private and unsafe functions if !it.is_visible_from(db, module) - || it.is_unsafe_to_call(db) + || it.is_unsafe_to_call(db, None) || it.is_unstable(db) || ctx.config.enable_borrowcheck && ret_ty.contains_reference(db) || ret_ty.is_raw_ptr() @@ -470,7 +470,10 @@ pub(super) fn impl_method<'a, DB: HirDatabase>( } // Filter out private and unsafe functions - if !it.is_visible_from(db, module) || it.is_unsafe_to_call(db) || it.is_unstable(db) { + if !it.is_visible_from(db, module) + || it.is_unsafe_to_call(db, None) + || it.is_unstable(db) + { return None; } @@ -658,7 +661,10 @@ pub(super) fn impl_static_method<'a, DB: HirDatabase>( } // Filter out private and unsafe functions - if !it.is_visible_from(db, module) || it.is_unsafe_to_call(db) || it.is_unstable(db) { + if !it.is_visible_from(db, module) + || it.is_unsafe_to_call(db, None) + || it.is_unstable(db) + { return None; } diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs index 2f1860cbb59a..7862b258789c 100644 --- a/crates/ide-completion/src/context.rs +++ b/crates/ide-completion/src/context.rs @@ -442,6 +442,8 @@ pub(crate) struct CompletionContext<'a> { pub(crate) krate: hir::Crate, /// The module of the `scope`. pub(crate) module: hir::Module, + /// The function where we're completing, if inside a function. + pub(crate) containing_function: Option, /// Whether nightly toolchain is used. Cached since this is looked up a lot. pub(crate) is_nightly: bool, /// The edition of the current crate @@ -760,6 +762,7 @@ impl<'a> CompletionContext<'a> { let krate = scope.krate(); let module = scope.module(); + let containing_function = scope.containing_function(); let edition = krate.edition(db); let toolchain = db.toolchain_channel(krate.into()); @@ -874,6 +877,7 @@ impl<'a> CompletionContext<'a> { token, krate, module, + containing_function, is_nightly, edition, expected_name, diff --git a/crates/ide-completion/src/render/function.rs b/crates/ide-completion/src/render/function.rs index c3354902c3b7..4931f8d09024 100644 --- a/crates/ide-completion/src/render/function.rs +++ b/crates/ide-completion/src/render/function.rs @@ -144,7 +144,7 @@ fn render( let detail = if ctx.completion.config.full_function_signatures { detail_full(db, func, ctx.completion.edition) } else { - detail(db, func, ctx.completion.edition) + detail(ctx.completion, func, ctx.completion.edition) }; item.set_documentation(ctx.docs(func)) .set_deprecated(ctx.is_deprecated(func) || ctx.is_deprecated_assoc_item(func)) @@ -307,26 +307,26 @@ fn ref_of_param(ctx: &CompletionContext<'_>, arg: &str, ty: &hir::Type) -> &'sta "" } -fn detail(db: &dyn HirDatabase, func: hir::Function, edition: Edition) -> String { - let mut ret_ty = func.ret_type(db); +fn detail(ctx: &CompletionContext<'_>, func: hir::Function, edition: Edition) -> String { + let mut ret_ty = func.ret_type(ctx.db); let mut detail = String::new(); - if func.is_const(db) { + if func.is_const(ctx.db) { format_to!(detail, "const "); } - if func.is_async(db) { + if func.is_async(ctx.db) { format_to!(detail, "async "); - if let Some(async_ret) = func.async_ret_type(db) { + if let Some(async_ret) = func.async_ret_type(ctx.db) { ret_ty = async_ret; } } - if func.is_unsafe_to_call(db) { + if func.is_unsafe_to_call(ctx.db, ctx.containing_function) { format_to!(detail, "unsafe "); } - format_to!(detail, "fn({})", params_display(db, func, edition)); + format_to!(detail, "fn({})", params_display(ctx.db, func, edition)); if !ret_ty.is_unit() { - format_to!(detail, " -> {}", ret_ty.display(db, edition)); + format_to!(detail, " -> {}", ret_ty.display(ctx.db, edition)); } detail } diff --git a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs index 8117401a5342..cea8bc5e9590 100644 --- a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs +++ b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs @@ -812,4 +812,24 @@ fn main() { "#, ) } + + #[test] + fn target_feature() { + check_diagnostics( + r#" +#[target_feature(enable = "avx")] +fn foo() {} + +#[target_feature(enable = "avx,avx2")] +fn bar() { + foo(); +} + +fn baz() { + foo(); + // ^^^^^ 💡 error: call to unsafe function is unsafe and requires an unsafe function or block +} + "#, + ); + } } diff --git a/crates/ide/src/syntax_highlighting/highlight.rs b/crates/ide/src/syntax_highlighting/highlight.rs index 22a2fe4e9eb2..a0c8b2856839 100644 --- a/crates/ide/src/syntax_highlighting/highlight.rs +++ b/crates/ide/src/syntax_highlighting/highlight.rs @@ -427,7 +427,11 @@ pub(super) fn highlight_def( } } - if func.is_unsafe_to_call(db) { + // FIXME: Passing `None` here means not-unsafe functions with `#[target_feature]` will be + // highlighted as unsafe, even when the current target features set is a superset (RFC 2396). + // We probably should consider checking the current function, but I found no easy way to do + // that (also I'm worried about perf). There's also an instance below. + if func.is_unsafe_to_call(db, None) { h |= HlMod::Unsafe; } if func.is_async(db) { @@ -589,7 +593,7 @@ fn highlight_method_call( let mut h = SymbolKind::Method.into(); - if func.is_unsafe_to_call(sema.db) || sema.is_unsafe_method_call(method_call) { + if func.is_unsafe_to_call(sema.db, None) || sema.is_unsafe_method_call(method_call) { h |= HlMod::Unsafe; } if func.is_async(sema.db) { diff --git a/crates/intern/src/symbol/symbols.rs b/crates/intern/src/symbol/symbols.rs index 9bc78ff87b8a..1b543ddf8110 100644 --- a/crates/intern/src/symbol/symbols.rs +++ b/crates/intern/src/symbol/symbols.rs @@ -457,6 +457,8 @@ define_symbols! { system, sysv64, Target, + target_feature, + enable, termination, test_case, test,