Support RFC 2396

AKA. target_feature 1.1, or non unsafe target_feature.
This commit is contained in:
Chayim Refael Friedman 2025-01-26 23:31:58 +02:00
parent 2df4ecfc74
commit 791b1ebfd4
14 changed files with 132 additions and 24 deletions

View File

@ -101,6 +101,10 @@ impl FunctionData {
flags.remove(FnFlags::HAS_UNSAFE_KW); flags.remove(FnFlags::HAS_UNSAFE_KW);
} }
if attrs.by_key(&sym::target_feature).exists() {
flags.insert(FnFlags::HAS_TARGET_FEATURE);
}
Arc::new(FunctionData { Arc::new(FunctionData {
name: func.name.clone(), name: func.name.clone(),
params: func params: func
@ -155,6 +159,10 @@ impl FunctionData {
pub fn is_varargs(&self) -> bool { pub fn is_varargs(&self) -> bool {
self.flags.contains(FnFlags::IS_VARARGS) 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]> { fn parse_rustc_legacy_const_generics(tt: &crate::tt::TopSubtree) -> Box<[u32]> {

View File

@ -937,7 +937,7 @@ pub struct Param {
bitflags::bitflags! { bitflags::bitflags! {
#[derive(Debug, Clone, Copy, Eq, PartialEq, Default)] #[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_SELF_PARAM = 1 << 0;
const HAS_BODY = 1 << 1; const HAS_BODY = 1 << 1;
const HAS_DEFAULT_KW = 1 << 2; const HAS_DEFAULT_KW = 1 << 2;
@ -946,6 +946,11 @@ bitflags::bitflags! {
const HAS_UNSAFE_KW = 1 << 5; const HAS_UNSAFE_KW = 1 << 5;
const IS_VARARGS = 1 << 6; const IS_VARARGS = 1 << 6;
const HAS_SAFE_KW = 1 << 7; 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;
} }
} }

View File

@ -14,7 +14,8 @@ use hir_def::{
}; };
use crate::{ 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)`. /// Returns `(unsafe_exprs, fn_is_unsafe)`.
@ -96,6 +97,7 @@ struct UnsafeVisitor<'a> {
inside_assignment: bool, inside_assignment: bool,
inside_union_destructure: bool, inside_union_destructure: bool,
unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason), unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason),
def_target_features: TargetFeatures,
} }
impl<'a> UnsafeVisitor<'a> { impl<'a> UnsafeVisitor<'a> {
@ -107,6 +109,10 @@ impl<'a> UnsafeVisitor<'a> {
unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason), unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason),
) -> Self { ) -> Self {
let resolver = def.resolver(db.upcast()); 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 { Self {
db, db,
infer, infer,
@ -117,6 +123,7 @@ impl<'a> UnsafeVisitor<'a> {
inside_assignment: false, inside_assignment: false,
inside_union_destructure: false, inside_union_destructure: false,
unsafe_expr_cb, unsafe_expr_cb,
def_target_features,
} }
} }
@ -181,7 +188,7 @@ impl<'a> UnsafeVisitor<'a> {
match expr { match expr {
&Expr::Call { callee, .. } => { &Expr::Call { callee, .. } => {
if let Some(func) = self.infer[callee].as_fn_def(self.db) { 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); self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall);
} }
} }
@ -212,7 +219,7 @@ impl<'a> UnsafeVisitor<'a> {
if self if self
.infer .infer
.method_resolution(current) .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) .unwrap_or(false)
{ {
self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall); self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall);

View File

@ -100,7 +100,7 @@ pub use mapping::{
}; };
pub use method_resolution::check_orphan_rules; pub use method_resolution::check_orphan_rules;
pub use traits::TraitEnvironment; 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 variance::Variance;
pub use chalk_ir::{ pub use chalk_ir::{

View File

@ -9,16 +9,18 @@ use chalk_ir::{
DebruijnIndex, DebruijnIndex,
}; };
use hir_def::{ use hir_def::{
attr::Attrs,
db::DefDatabase, db::DefDatabase,
generics::{WherePredicate, WherePredicateTypeTarget}, generics::{WherePredicate, WherePredicateTypeTarget},
lang_item::LangItem, lang_item::LangItem,
resolver::{HasResolver, TypeNs}, resolver::{HasResolver, TypeNs},
tt,
type_ref::{TraitBoundModifier, TypeRef}, type_ref::{TraitBoundModifier, TypeRef},
EnumId, EnumVariantId, FunctionId, Lookup, OpaqueInternableThing, TraitId, TypeAliasId, EnumId, EnumVariantId, FunctionId, Lookup, OpaqueInternableThing, TraitId, TypeAliasId,
TypeOrConstParamId, TypeOrConstParamId,
}; };
use hir_expand::name::Name; use hir_expand::name::Name;
use intern::sym; use intern::{sym, Symbol};
use rustc_abi::TargetDataLayout; use rustc_abi::TargetDataLayout;
use rustc_hash::FxHashSet; use rustc_hash::FxHashSet;
use smallvec::{smallvec, SmallVec}; 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<Symbol>,
}
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); let data = db.function_data(func);
if data.is_unsafe() { if data.is_unsafe() {
return true; return true;
} }
if data.has_target_feature() {
// RFC 2396 <https://rust-lang.github.io/rfcs/2396-target-feature-1.1.html>.
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()); let loc = func.lookup(db.upcast());
match loc.container { match loc.container {
hir_def::ItemContainerId::ExternBlockId(block) => { hir_def::ItemContainerId::ExternBlockId(block) => {

View File

@ -80,7 +80,9 @@ impl HirDisplay for Function {
if data.is_async() { if data.is_async() {
f.write_str("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 ")?; f.write_str("unsafe ")?;
} }
if let Some(abi) = &data.abi { if let Some(abi) = &data.abi {

View File

@ -2361,8 +2361,11 @@ impl Function {
db.attrs(self.id.into()).is_unstable() db.attrs(self.id.into()).is_unstable()
} }
pub fn is_unsafe_to_call(self, db: &dyn HirDatabase) -> bool { pub fn is_unsafe_to_call(self, db: &dyn HirDatabase, caller: Option<Function>) -> bool {
hir_ty::is_fn_unsafe_to_call(db, self.id) 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. /// Whether this function declaration has a definition.

View File

@ -2040,6 +2040,13 @@ impl SemanticsScope<'_> {
Crate { id: self.resolver.krate() } Crate { id: self.resolver.krate() }
} }
pub fn containing_function(&self) -> Option<Function> {
self.resolver.body_owner().and_then(|owner| match owner {
DefWithBodyId::FunctionId(id) => Some(id.into()),
_ => None,
})
}
pub(crate) fn resolver(&self) -> &Resolver { pub(crate) fn resolver(&self) -> &Resolver {
&self.resolver &self.resolver
} }

View File

@ -365,7 +365,7 @@ pub(super) fn free_function<'a, DB: HirDatabase>(
let ret_ty = it.ret_type_with_args(db, generics.iter().cloned()); let ret_ty = it.ret_type_with_args(db, generics.iter().cloned());
// Filter out private and unsafe functions // Filter out private and unsafe functions
if !it.is_visible_from(db, module) if !it.is_visible_from(db, module)
|| it.is_unsafe_to_call(db) || it.is_unsafe_to_call(db, None)
|| it.is_unstable(db) || it.is_unstable(db)
|| ctx.config.enable_borrowcheck && ret_ty.contains_reference(db) || ctx.config.enable_borrowcheck && ret_ty.contains_reference(db)
|| ret_ty.is_raw_ptr() || ret_ty.is_raw_ptr()
@ -470,7 +470,10 @@ pub(super) fn impl_method<'a, DB: HirDatabase>(
} }
// Filter out private and unsafe functions // 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; return None;
} }
@ -658,7 +661,10 @@ pub(super) fn impl_static_method<'a, DB: HirDatabase>(
} }
// Filter out private and unsafe functions // 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; return None;
} }

View File

@ -442,6 +442,8 @@ pub(crate) struct CompletionContext<'a> {
pub(crate) krate: hir::Crate, pub(crate) krate: hir::Crate,
/// The module of the `scope`. /// The module of the `scope`.
pub(crate) module: hir::Module, pub(crate) module: hir::Module,
/// The function where we're completing, if inside a function.
pub(crate) containing_function: Option<hir::Function>,
/// Whether nightly toolchain is used. Cached since this is looked up a lot. /// Whether nightly toolchain is used. Cached since this is looked up a lot.
pub(crate) is_nightly: bool, pub(crate) is_nightly: bool,
/// The edition of the current crate /// The edition of the current crate
@ -760,6 +762,7 @@ impl<'a> CompletionContext<'a> {
let krate = scope.krate(); let krate = scope.krate();
let module = scope.module(); let module = scope.module();
let containing_function = scope.containing_function();
let edition = krate.edition(db); let edition = krate.edition(db);
let toolchain = db.toolchain_channel(krate.into()); let toolchain = db.toolchain_channel(krate.into());
@ -874,6 +877,7 @@ impl<'a> CompletionContext<'a> {
token, token,
krate, krate,
module, module,
containing_function,
is_nightly, is_nightly,
edition, edition,
expected_name, expected_name,

View File

@ -144,7 +144,7 @@ fn render(
let detail = if ctx.completion.config.full_function_signatures { let detail = if ctx.completion.config.full_function_signatures {
detail_full(db, func, ctx.completion.edition) detail_full(db, func, ctx.completion.edition)
} else { } else {
detail(db, func, ctx.completion.edition) detail(ctx.completion, func, ctx.completion.edition)
}; };
item.set_documentation(ctx.docs(func)) item.set_documentation(ctx.docs(func))
.set_deprecated(ctx.is_deprecated(func) || ctx.is_deprecated_assoc_item(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 { fn detail(ctx: &CompletionContext<'_>, func: hir::Function, edition: Edition) -> String {
let mut ret_ty = func.ret_type(db); let mut ret_ty = func.ret_type(ctx.db);
let mut detail = String::new(); let mut detail = String::new();
if func.is_const(db) { if func.is_const(ctx.db) {
format_to!(detail, "const "); format_to!(detail, "const ");
} }
if func.is_async(db) { if func.is_async(ctx.db) {
format_to!(detail, "async "); 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; 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, "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() { if !ret_ty.is_unit() {
format_to!(detail, " -> {}", ret_ty.display(db, edition)); format_to!(detail, " -> {}", ret_ty.display(ctx.db, edition));
} }
detail detail
} }

View File

@ -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
}
"#,
);
}
} }

View File

@ -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; h |= HlMod::Unsafe;
} }
if func.is_async(db) { if func.is_async(db) {
@ -589,7 +593,7 @@ fn highlight_method_call(
let mut h = SymbolKind::Method.into(); 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; h |= HlMod::Unsafe;
} }
if func.is_async(sema.db) { if func.is_async(sema.db) {

View File

@ -457,6 +457,8 @@ define_symbols! {
system, system,
sysv64, sysv64,
Target, Target,
target_feature,
enable,
termination, termination,
test_case, test_case,
test, test,