Skip to content

Commit 9051647

Browse files
VeykrilVeykril
authored
Merge pull request rust-lang#19038 from ChayimFriedman2/unused-unsafe
feat: Support RFC 2396
2 parents a7cbe4b + 5de2cd4 commit 9051647

File tree

14 files changed

+132
-24
lines changed

14 files changed

+132
-24
lines changed

‎src/tools/rust-analyzer/crates/hir-def/src/data.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ impl FunctionData {
101101
flags.remove(FnFlags::HAS_UNSAFE_KW);
102102
}
103103

104+
if attrs.by_key(&sym::target_feature).exists() {
105+
flags.insert(FnFlags::HAS_TARGET_FEATURE);
106+
}
107+
104108
Arc::new(FunctionData {
105109
name: func.name.clone(),
106110
params: func
@@ -155,6 +159,10 @@ impl FunctionData {
155159
pub fn is_varargs(&self) -> bool {
156160
self.flags.contains(FnFlags::IS_VARARGS)
157161
}
162+
163+
pub fn has_target_feature(&self) -> bool {
164+
self.flags.contains(FnFlags::HAS_TARGET_FEATURE)
165+
}
158166
}
159167

160168
fn parse_rustc_legacy_const_generics(tt: &crate::tt::TopSubtree) -> Box<[u32]> {

‎src/tools/rust-analyzer/crates/hir-def/src/item_tree.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -937,7 +937,7 @@ pub struct Param {
937937

938938
bitflags::bitflags! {
939939
#[derive(Debug, Clone, Copy, Eq, PartialEq, Default)]
940-
pub(crate) struct FnFlags: u8 {
940+
pub(crate) struct FnFlags: u16 {
941941
const HAS_SELF_PARAM = 1 << 0;
942942
const HAS_BODY = 1 << 1;
943943
const HAS_DEFAULT_KW = 1 << 2;
@@ -946,6 +946,11 @@ bitflags::bitflags! {
946946
const HAS_UNSAFE_KW = 1 << 5;
947947
const IS_VARARGS = 1 << 6;
948948
const HAS_SAFE_KW = 1 << 7;
949+
/// The `#[target_feature]` attribute is necessary to check safety (with RFC 2396),
950+
/// but keeping it for all functions will consume a lot of memory when there are
951+
/// only very few functions with it. So we only encode its existence here, and lookup
952+
/// it if needed.
953+
const HAS_TARGET_FEATURE = 1 << 8;
949954
}
950955
}
951956

‎src/tools/rust-analyzer/crates/hir-ty/src/diagnostics/unsafe_check.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ use hir_def::{
1414
};
1515

1616
use crate::{
17-
db::HirDatabase, utils::is_fn_unsafe_to_call, InferenceResult, Interner, TyExt, TyKind,
17+
db::HirDatabase, utils::is_fn_unsafe_to_call, InferenceResult, Interner, TargetFeatures, TyExt,
18+
TyKind,
1819
};
1920

2021
/// Returns `(unsafe_exprs, fn_is_unsafe)`.
@@ -96,6 +97,7 @@ struct UnsafeVisitor<'a> {
9697
inside_assignment: bool,
9798
inside_union_destructure: bool,
9899
unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason),
100+
def_target_features: TargetFeatures,
99101
}
100102

101103
impl<'a> UnsafeVisitor<'a> {
@@ -107,6 +109,10 @@ impl<'a> UnsafeVisitor<'a> {
107109
unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason),
108110
) -> Self {
109111
let resolver = def.resolver(db.upcast());
112+
let def_target_features = match def {
113+
DefWithBodyId::FunctionId(func) => TargetFeatures::from_attrs(&db.attrs(func.into())),
114+
_ => TargetFeatures::default(),
115+
};
110116
Self {
111117
db,
112118
infer,
@@ -117,6 +123,7 @@ impl<'a> UnsafeVisitor<'a> {
117123
inside_assignment: false,
118124
inside_union_destructure: false,
119125
unsafe_expr_cb,
126+
def_target_features,
120127
}
121128
}
122129

@@ -181,7 +188,7 @@ impl<'a> UnsafeVisitor<'a> {
181188
match expr {
182189
&Expr::Call { callee, .. } => {
183190
if let Some(func) = self.infer[callee].as_fn_def(self.db) {
184-
if is_fn_unsafe_to_call(self.db, func) {
191+
if is_fn_unsafe_to_call(self.db, func, &self.def_target_features) {
185192
self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall);
186193
}
187194
}
@@ -212,7 +219,7 @@ impl<'a> UnsafeVisitor<'a> {
212219
if self
213220
.infer
214221
.method_resolution(current)
215-
.map(|(func, _)| is_fn_unsafe_to_call(self.db, func))
222+
.map(|(func, _)| is_fn_unsafe_to_call(self.db, func, &self.def_target_features))
216223
.unwrap_or(false)
217224
{
218225
self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall);

‎src/tools/rust-analyzer/crates/hir-ty/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ pub use mapping::{
100100
};
101101
pub use method_resolution::check_orphan_rules;
102102
pub use traits::TraitEnvironment;
103-
pub use utils::{all_super_traits, direct_super_traits, is_fn_unsafe_to_call};
103+
pub use utils::{all_super_traits, direct_super_traits, is_fn_unsafe_to_call, TargetFeatures};
104104
pub use variance::Variance;
105105

106106
pub use chalk_ir::{

‎src/tools/rust-analyzer/crates/hir-ty/src/utils.rs

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,18 @@ use chalk_ir::{
99
DebruijnIndex,
1010
};
1111
use hir_def::{
12+
attr::Attrs,
1213
db::DefDatabase,
1314
generics::{WherePredicate, WherePredicateTypeTarget},
1415
lang_item::LangItem,
1516
resolver::{HasResolver, TypeNs},
17+
tt,
1618
type_ref::{TraitBoundModifier, TypeRef},
1719
EnumId, EnumVariantId, FunctionId, Lookup, OpaqueInternableThing, TraitId, TypeAliasId,
1820
TypeOrConstParamId,
1921
};
2022
use hir_expand::name::Name;
21-
use intern::sym;
23+
use intern::{sym, Symbol};
2224
use rustc_abi::TargetDataLayout;
2325
use rustc_hash::FxHashSet;
2426
use smallvec::{smallvec, SmallVec};
@@ -264,12 +266,50 @@ impl<'a> ClosureSubst<'a> {
264266
}
265267
}
266268

267-
pub fn is_fn_unsafe_to_call(db: &dyn HirDatabase, func: FunctionId) -> bool {
269+
#[derive(Debug, Default)]
270+
pub struct TargetFeatures {
271+
enabled: FxHashSet<Symbol>,
272+
}
273+
274+
impl TargetFeatures {
275+
pub fn from_attrs(attrs: &Attrs) -> Self {
276+
let enabled = attrs
277+
.by_key(&sym::target_feature)
278+
.tt_values()
279+
.filter_map(|tt| {
280+
match tt.token_trees().flat_tokens() {
281+
[
282+
tt::TokenTree::Leaf(tt::Leaf::Ident(enable_ident)),
283+
tt::TokenTree::Leaf(tt::Leaf::Punct(tt::Punct { char: '=', .. })),
284+
tt::TokenTree::Leaf(tt::Leaf::Literal(tt::Literal { kind: tt::LitKind::Str, symbol: features, .. })),
285+
] if enable_ident.sym == sym::enable => Some(features),
286+
_ => None,
287+
}
288+
})
289+
.flat_map(|features| features.as_str().split(',').map(Symbol::intern))
290+
.collect();
291+
Self { enabled }
292+
}
293+
}
294+
295+
pub fn is_fn_unsafe_to_call(
296+
db: &dyn HirDatabase,
297+
func: FunctionId,
298+
caller_target_features: &TargetFeatures,
299+
) -> bool {
268300
let data = db.function_data(func);
269301
if data.is_unsafe() {
270302
return true;
271303
}
272304

305+
if data.has_target_feature() {
306+
// RFC 2396 <https://rust-lang.github.io/rfcs/2396-target-feature-1.1.html>.
307+
let callee_target_features = TargetFeatures::from_attrs(&db.attrs(func.into()));
308+
if !caller_target_features.enabled.is_superset(&callee_target_features.enabled) {
309+
return true;
310+
}
311+
}
312+
273313
let loc = func.lookup(db.upcast());
274314
match loc.container {
275315
hir_def::ItemContainerId::ExternBlockId(block) => {

‎src/tools/rust-analyzer/crates/hir/src/display.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ impl HirDisplay for Function {
8080
if data.is_async() {
8181
f.write_str("async ")?;
8282
}
83-
if self.is_unsafe_to_call(db) {
83+
// FIXME: This will show `unsafe` for functions that are `#[target_feature]` but not unsafe
84+
// (they are conditionally unsafe to call). We probably should show something else.
85+
if self.is_unsafe_to_call(db, None) {
8486
f.write_str("unsafe ")?;
8587
}
8688
if let Some(abi) = &data.abi {

‎src/tools/rust-analyzer/crates/hir/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2361,8 +2361,11 @@ impl Function {
23612361
db.attrs(self.id.into()).is_unstable()
23622362
}
23632363

2364-
pub fn is_unsafe_to_call(self, db: &dyn HirDatabase) -> bool {
2365-
hir_ty::is_fn_unsafe_to_call(db, self.id)
2364+
pub fn is_unsafe_to_call(self, db: &dyn HirDatabase, caller: Option<Function>) -> bool {
2365+
let target_features = caller
2366+
.map(|caller| hir_ty::TargetFeatures::from_attrs(&db.attrs(caller.id.into())))
2367+
.unwrap_or_default();
2368+
hir_ty::is_fn_unsafe_to_call(db, self.id, &target_features)
23662369
}
23672370

23682371
/// Whether this function declaration has a definition.

‎src/tools/rust-analyzer/crates/hir/src/semantics.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2040,6 +2040,13 @@ impl SemanticsScope<'_> {
20402040
Crate { id: self.resolver.krate() }
20412041
}
20422042

2043+
pub fn containing_function(&self) -> Option<Function> {
2044+
self.resolver.body_owner().and_then(|owner| match owner {
2045+
DefWithBodyId::FunctionId(id) => Some(id.into()),
2046+
_ => None,
2047+
})
2048+
}
2049+
20432050
pub(crate) fn resolver(&self) -> &Resolver {
20442051
&self.resolver
20452052
}

‎src/tools/rust-analyzer/crates/hir/src/term_search/tactics.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ pub(super) fn free_function<'a, DB: HirDatabase>(
365365
let ret_ty = it.ret_type_with_args(db, generics.iter().cloned());
366366
// Filter out private and unsafe functions
367367
if !it.is_visible_from(db, module)
368-
|| it.is_unsafe_to_call(db)
368+
|| it.is_unsafe_to_call(db, None)
369369
|| it.is_unstable(db)
370370
|| ctx.config.enable_borrowcheck && ret_ty.contains_reference(db)
371371
|| ret_ty.is_raw_ptr()
@@ -470,7 +470,10 @@ pub(super) fn impl_method<'a, DB: HirDatabase>(
470470
}
471471

472472
// Filter out private and unsafe functions
473-
if !it.is_visible_from(db, module) || it.is_unsafe_to_call(db) || it.is_unstable(db) {
473+
if !it.is_visible_from(db, module)
474+
|| it.is_unsafe_to_call(db, None)
475+
|| it.is_unstable(db)
476+
{
474477
return None;
475478
}
476479

@@ -658,7 +661,10 @@ pub(super) fn impl_static_method<'a, DB: HirDatabase>(
658661
}
659662

660663
// Filter out private and unsafe functions
661-
if !it.is_visible_from(db, module) || it.is_unsafe_to_call(db) || it.is_unstable(db) {
664+
if !it.is_visible_from(db, module)
665+
|| it.is_unsafe_to_call(db, None)
666+
|| it.is_unstable(db)
667+
{
662668
return None;
663669
}
664670

‎src/tools/rust-analyzer/crates/ide-completion/src/context.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,8 @@ pub(crate) struct CompletionContext<'a> {
442442
pub(crate) krate: hir::Crate,
443443
/// The module of the `scope`.
444444
pub(crate) module: hir::Module,
445+
/// The function where we're completing, if inside a function.
446+
pub(crate) containing_function: Option<hir::Function>,
445447
/// Whether nightly toolchain is used. Cached since this is looked up a lot.
446448
pub(crate) is_nightly: bool,
447449
/// The edition of the current crate
@@ -760,6 +762,7 @@ impl<'a> CompletionContext<'a> {
760762

761763
let krate = scope.krate();
762764
let module = scope.module();
765+
let containing_function = scope.containing_function();
763766
let edition = krate.edition(db);
764767

765768
let toolchain = db.toolchain_channel(krate.into());
@@ -874,6 +877,7 @@ impl<'a> CompletionContext<'a> {
874877
token,
875878
krate,
876879
module,
880+
containing_function,
877881
is_nightly,
878882
edition,
879883
expected_name,

‎src/tools/rust-analyzer/crates/ide-completion/src/render/function.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ fn render(
144144
let detail = if ctx.completion.config.full_function_signatures {
145145
detail_full(db, func, ctx.completion.edition)
146146
} else {
147-
detail(db, func, ctx.completion.edition)
147+
detail(ctx.completion, func, ctx.completion.edition)
148148
};
149149
item.set_documentation(ctx.docs(func))
150150
.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
307307
""
308308
}
309309

310-
fn detail(db: &dyn HirDatabase, func: hir::Function, edition: Edition) -> String {
311-
let mut ret_ty = func.ret_type(db);
310+
fn detail(ctx: &CompletionContext<'_>, func: hir::Function, edition: Edition) -> String {
311+
let mut ret_ty = func.ret_type(ctx.db);
312312
let mut detail = String::new();
313313

314-
if func.is_const(db) {
314+
if func.is_const(ctx.db) {
315315
format_to!(detail, "const ");
316316
}
317-
if func.is_async(db) {
317+
if func.is_async(ctx.db) {
318318
format_to!(detail, "async ");
319-
if let Some(async_ret) = func.async_ret_type(db) {
319+
if let Some(async_ret) = func.async_ret_type(ctx.db) {
320320
ret_ty = async_ret;
321321
}
322322
}
323-
if func.is_unsafe_to_call(db) {
323+
if func.is_unsafe_to_call(ctx.db, ctx.containing_function) {
324324
format_to!(detail, "unsafe ");
325325
}
326326

327-
format_to!(detail, "fn({})", params_display(db, func, edition));
327+
format_to!(detail, "fn({})", params_display(ctx.db, func, edition));
328328
if !ret_ty.is_unit() {
329-
format_to!(detail, " -> {}", ret_ty.display(db, edition));
329+
format_to!(detail, " -> {}", ret_ty.display(ctx.db, edition));
330330
}
331331
detail
332332
}

‎src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_unsafe.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,4 +812,24 @@ fn main() {
812812
"#,
813813
)
814814
}
815+
816+
#[test]
817+
fn target_feature() {
818+
check_diagnostics(
819+
r#"
820+
#[target_feature(enable = "avx")]
821+
fn foo() {}
822+
823+
#[target_feature(enable = "avx,avx2")]
824+
fn bar() {
825+
foo();
826+
}
827+
828+
fn baz() {
829+
foo();
830+
// ^^^^^ 💡 error: call to unsafe function is unsafe and requires an unsafe function or block
831+
}
832+
"#,
833+
);
834+
}
815835
}

‎src/tools/rust-analyzer/crates/ide/src/syntax_highlighting/highlight.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,11 @@ pub(super) fn highlight_def(
427427
}
428428
}
429429

430-
if func.is_unsafe_to_call(db) {
430+
// FIXME: Passing `None` here means not-unsafe functions with `#[target_feature]` will be
431+
// highlighted as unsafe, even when the current target features set is a superset (RFC 2396).
432+
// We probably should consider checking the current function, but I found no easy way to do
433+
// that (also I'm worried about perf). There's also an instance below.
434+
if func.is_unsafe_to_call(db, None) {
431435
h |= HlMod::Unsafe;
432436
}
433437
if func.is_async(db) {
@@ -589,7 +593,7 @@ fn highlight_method_call(
589593

590594
let mut h = SymbolKind::Method.into();
591595

592-
if func.is_unsafe_to_call(sema.db) || sema.is_unsafe_method_call(method_call) {
596+
if func.is_unsafe_to_call(sema.db, None) || sema.is_unsafe_method_call(method_call) {
593597
h |= HlMod::Unsafe;
594598
}
595599
if func.is_async(sema.db) {

‎src/tools/rust-analyzer/crates/intern/src/symbol/symbols.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,8 @@ define_symbols! {
457457
system,
458458
sysv64,
459459
Target,
460+
target_feature,
461+
enable,
460462
termination,
461463
test_case,
462464
test,

0 commit comments

Comments
 (0)