Skip to content

Commit

Permalink
Auto merge of #18350 - roife:safe-kw-1, r=lnicola
Browse files Browse the repository at this point in the history
feat: initial support for safe_kw in extern blocks

This PR adds initial support for `safe` keywords in external blocks.

## Changes

1. Parsing static declarations with `safe` kw and `unsafe` kw, as well as functions with `safe` kw in extern_blocks
2. Add `HAS_SAFE_KW ` to `FnFlags`
3. Handle `safe` kw in `is_fn_unsafe_to_call` query
4. Handle safe_kw in unsafe diagnostics
  • Loading branch information
bors committed Oct 20, 2024
2 parents 687b72c + 002f6ad commit 6dc5b32
Show file tree
Hide file tree
Showing 13 changed files with 326 additions and 12 deletions.
8 changes: 8 additions & 0 deletions crates/hir-def/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ impl FunctionData {
self.flags.contains(FnFlags::HAS_UNSAFE_KW)
}

pub fn is_safe(&self) -> bool {
self.flags.contains(FnFlags::HAS_SAFE_KW)
}

pub fn is_varargs(&self) -> bool {
self.flags.contains(FnFlags::IS_VARARGS)
}
Expand Down Expand Up @@ -567,6 +571,8 @@ pub struct StaticData {
pub visibility: RawVisibility,
pub mutable: bool,
pub is_extern: bool,
pub has_safe_kw: bool,
pub has_unsafe_kw: bool,
}

impl StaticData {
Expand All @@ -581,6 +587,8 @@ impl StaticData {
visibility: item_tree[statik.visibility].clone(),
mutable: statik.mutable,
is_extern: matches!(loc.container, ItemContainerId::ExternBlockId(_)),
has_safe_kw: statik.has_safe_kw,
has_unsafe_kw: statik.has_unsafe_kw,
})
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/hir-def/src/item_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,7 @@ bitflags::bitflags! {
const HAS_ASYNC_KW = 1 << 4;
const HAS_UNSAFE_KW = 1 << 5;
const IS_VARARGS = 1 << 6;
const HAS_SAFE_KW = 1 << 7;
}
}

Expand Down Expand Up @@ -822,7 +823,10 @@ pub struct Const {
pub struct Static {
pub name: Name,
pub visibility: RawVisibilityId,
// TODO: use bitflags when we have more flags
pub mutable: bool,
pub has_safe_kw: bool,
pub has_unsafe_kw: bool,
pub type_ref: Interned<TypeRef>,
pub ast_id: FileAstId<ast::Static>,
}
Expand Down
8 changes: 7 additions & 1 deletion crates/hir-def/src/item_tree/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,9 @@ impl<'a> Ctx<'a> {
if func.unsafe_token().is_some() {
flags |= FnFlags::HAS_UNSAFE_KW;
}
if func.safe_token().is_some() {
flags |= FnFlags::HAS_SAFE_KW;
}
if has_var_args {
flags |= FnFlags::IS_VARARGS;
}
Expand Down Expand Up @@ -484,8 +487,11 @@ impl<'a> Ctx<'a> {
let type_ref = self.lower_type_ref_opt(static_.ty());
let visibility = self.lower_visibility(static_);
let mutable = static_.mut_token().is_some();
let has_safe_kw = static_.safe_token().is_some();
let has_unsafe_kw = static_.unsafe_token().is_some();
let ast_id = self.source_ast_id_map.ast_id(static_);
let res = Static { name, visibility, mutable, type_ref, ast_id };
let res =
Static { name, visibility, mutable, type_ref, ast_id, has_safe_kw, has_unsafe_kw };
Some(id(self.data().statics.alloc(res)))
}

Expand Down
19 changes: 18 additions & 1 deletion crates/hir-def/src/item_tree/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,9 @@ impl Printer<'_> {
if flags.contains(FnFlags::HAS_UNSAFE_KW) {
w!(self, "unsafe ");
}
if flags.contains(FnFlags::HAS_SAFE_KW) {
w!(self, "safe ");
}
if let Some(abi) = abi {
w!(self, "extern \"{}\" ", abi);
}
Expand Down Expand Up @@ -379,9 +382,23 @@ impl Printer<'_> {
wln!(self, " = _;");
}
ModItem::Static(it) => {
let Static { name, visibility, mutable, type_ref, ast_id } = &self.tree[it];
let Static {
name,
visibility,
mutable,
type_ref,
ast_id,
has_safe_kw,
has_unsafe_kw,
} = &self.tree[it];
self.print_ast_id(ast_id.erase());
self.print_visibility(*visibility);
if *has_safe_kw {
w!(self, "safe ");
}
if *has_unsafe_kw {
w!(self, "unsafe ");
}
w!(self, "static ");
if *mutable {
w!(self, "mut ");
Expand Down
2 changes: 1 addition & 1 deletion crates/hir-ty/src/diagnostics/unsafe_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ fn walk_unsafe(
let value_or_partial = resolver.resolve_path_in_value_ns(db.upcast(), path);
if let Some(ResolveValueResult::ValueNs(ValueNs::StaticId(id), _)) = value_or_partial {
let static_data = db.static_data(id);
if static_data.mutable || static_data.is_extern {
if static_data.mutable || (static_data.is_extern && !static_data.has_safe_kw) {
unsafe_expr_cb(UnsafeExpr { expr: current, inside_unsafe_block });
}
}
Expand Down
12 changes: 7 additions & 5 deletions crates/hir-ty/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,12 @@ pub fn is_fn_unsafe_to_call(db: &dyn HirDatabase, func: FunctionId) -> bool {
return true;
}

match func.lookup(db.upcast()).container {
let loc = func.lookup(db.upcast());
match loc.container {
hir_def::ItemContainerId::ExternBlockId(block) => {
// Function in an `extern` block are always unsafe to call, except when it has
// `"rust-intrinsic"` ABI there are a few exceptions.
// Function in an `extern` block are always unsafe to call, except when
// it is marked as `safe` or it has `"rust-intrinsic"` ABI there are a
// few exceptions.
let id = block.lookup(db.upcast()).id;

let is_intrinsic =
Expand All @@ -270,8 +272,8 @@ pub fn is_fn_unsafe_to_call(db: &dyn HirDatabase, func: FunctionId) -> bool {
// Intrinsics are unsafe unless they have the rustc_safe_intrinsic attribute
!data.attrs.by_key(&sym::rustc_safe_intrinsic).exists()
} else {
// Extern items are always unsafe
true
// Extern items without `safe` modifier are always unsafe
!db.function_data(func).is_safe()
}
}
_ => false,
Expand Down
39 changes: 37 additions & 2 deletions crates/ide-diagnostics/src/handlers/missing_unsafe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ fn main() {
r#"
//- /ed2021.rs crate:ed2021 edition:2021
#[rustc_deprecated_safe_2024]
unsafe fn safe() -> u8 {
unsafe fn safe_fn() -> u8 {
0
}
//- /ed2024.rs crate:ed2024 edition:2024
Expand All @@ -564,7 +564,7 @@ unsafe fn not_safe() -> u8 {
}
//- /main.rs crate:main deps:ed2021,ed2024
fn main() {
ed2021::safe();
ed2021::safe_fn();
ed2024::not_safe();
//^^^^^^^^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block
}
Expand Down Expand Up @@ -595,4 +595,39 @@ unsafe fn foo(p: *mut i32) {
"#,
)
}

#[test]
fn no_unsafe_diagnostic_with_safe_kw() {
check_diagnostics(
r#"
unsafe extern {
pub safe fn f();
pub unsafe fn g();
pub fn h();
pub safe static S1: i32;
pub unsafe static S2: i32;
pub static S3: i32;
}
fn main() {
f();
g();
//^^^💡 error: this operation is unsafe and requires an unsafe function or block
h();
//^^^💡 error: this operation is unsafe and requires an unsafe function or block
let _ = S1;
let _ = S2;
//^^💡 error: this operation is unsafe and requires an unsafe function or block
let _ = S3;
//^^💡 error: this operation is unsafe and requires an unsafe function or block
}
"#,
);
}
}
6 changes: 6 additions & 0 deletions crates/parser/src/grammar/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ pub(super) fn opt_item(p: &mut Parser<'_>, m: Marker) -> Result<(), Marker> {
has_mods = true;
}

if p.at(T![safe]) {
p.eat(T![safe]);
has_mods = true;
}

if p.at(T![extern]) {
has_extern = true;
has_mods = true;
Expand Down Expand Up @@ -189,6 +194,7 @@ pub(super) fn opt_item(p: &mut Parser<'_>, m: Marker) -> Result<(), Marker> {
T![fn] => fn_(p, m),

T![const] if p.nth(1) != T!['{'] => consts::konst(p, m),
T![static] if matches!(p.nth(1), IDENT | T![_] | T![mut]) => consts::static_(p, m),

T![trait] => traits::trait_(p, m),
T![impl] => traits::impl_(p, m),
Expand Down
Loading

0 comments on commit 6dc5b32

Please sign in to comment.