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

Add new safety enum for inner extern items #124455

Closed
wants to merge 2 commits into from
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
23 changes: 17 additions & 6 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2105,7 +2105,7 @@ impl Ty {

#[derive(Clone, Encodable, Decodable, Debug)]
pub struct BareFnTy {
pub unsafety: Unsafe,
pub safety: Safety,
pub ext: Extern,
pub generic_params: ThinVec<GenericParam>,
pub decl: P<FnDecl>,
Expand Down Expand Up @@ -2491,6 +2491,17 @@ pub enum Unsafe {
No,
}

/// Safety of items (for now only used on inner extern block items).
#[derive(Copy, Clone, PartialEq, Eq, Hash, Encodable, Decodable, Debug)]
#[derive(HashStable_Generic)]
pub enum Safety {
/// `unsafe` an item is explicitly marked as `unsafe`.
Unsafe(Span),
/// Default means no value was provided, it will take a default value given the context in
/// which is used.
Default,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe name this Inherited or FromContext. Without an explanation it's not obvious what this is supposed to be

Copy link
Member Author

@spastorino spastorino Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've used Default as a way of saying no safety explicitly written means is going to take a default value.
Another possible name could be Implicit.
Out of all the values mentioned I slightly prefer Default but I don't mind changing it neither.
Another name we are using is Normal which I also think is not great, but it is another option.
Let's see if somebody else have some thought about this.

}

/// Describes what kind of coroutine markers, if any, a function has.
///
/// Coroutine markers are things that cause the function to generate a coroutine, such as `async`,
Expand Down Expand Up @@ -3011,8 +3022,8 @@ impl Extern {
/// included in this struct (e.g., `async unsafe fn` or `const extern "C" fn`).
#[derive(Clone, Copy, Encodable, Decodable, Debug)]
pub struct FnHeader {
/// The `unsafe` keyword, if any
pub unsafety: Unsafe,
/// The safety keyword, if any
pub safety: Safety,
/// Whether this is `async`, `gen`, or nothing.
pub coroutine_kind: Option<CoroutineKind>,
/// The `const` keyword, if any
Expand All @@ -3024,8 +3035,8 @@ pub struct FnHeader {
impl FnHeader {
/// Does this function header have any qualifiers or is it empty?
pub fn has_qualifiers(&self) -> bool {
let Self { unsafety, coroutine_kind, constness, ext } = self;
matches!(unsafety, Unsafe::Yes(_))
let Self { safety, coroutine_kind, constness, ext } = self;
matches!(safety, Safety::Unsafe(_))
|| coroutine_kind.is_some()
|| matches!(constness, Const::Yes(_))
|| !matches!(ext, Extern::None)
Expand All @@ -3035,7 +3046,7 @@ impl FnHeader {
impl Default for FnHeader {
fn default() -> FnHeader {
FnHeader {
unsafety: Unsafe::No,
safety: Safety::Default,
coroutine_kind: None,
constness: Const::No,
ext: Extern::None,
Expand Down
15 changes: 11 additions & 4 deletions compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,8 @@ pub fn noop_visit_ty<T: MutVisitor>(ty: &mut P<Ty>, vis: &mut T) {
vis.visit_mt(mt);
}
TyKind::BareFn(bft) => {
let BareFnTy { unsafety, ext: _, generic_params, decl, decl_span } = bft.deref_mut();
visit_unsafety(unsafety, vis);
let BareFnTy { safety, ext: _, generic_params, decl, decl_span } = bft.deref_mut();
visit_safety(safety, vis);
generic_params.flat_map_in_place(|param| vis.flat_map_generic_param(param));
vis.visit_fn_decl(decl);
vis.visit_span(decl_span);
Expand Down Expand Up @@ -866,6 +866,13 @@ fn visit_unsafety<T: MutVisitor>(unsafety: &mut Unsafe, vis: &mut T) {
}
}

fn visit_safety<T: MutVisitor>(safety: &mut Safety, vis: &mut T) {
match safety {
Safety::Unsafe(span) => vis.visit_span(span),
Safety::Default => {}
}
}

// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
fn visit_polarity<T: MutVisitor>(polarity: &mut ImplPolarity, vis: &mut T) {
match polarity {
Expand Down Expand Up @@ -1254,10 +1261,10 @@ fn visit_const_item<T: MutVisitor>(
}

fn noop_visit_fn_header<T: MutVisitor>(header: &mut FnHeader, vis: &mut T) {
let FnHeader { unsafety, coroutine_kind, constness, ext: _ } = header;
let FnHeader { safety, coroutine_kind, constness, ext: _ } = header;
visit_constness(constness, vis);
coroutine_kind.as_mut().map(|coroutine_kind| vis.visit_coroutine_kind(coroutine_kind));
visit_unsafety(unsafety, vis);
visit_safety(safety, vis);
}

pub fn noop_visit_crate<T: MutVisitor>(krate: &mut Crate, vis: &mut T) {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_lowering/src/delegation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
Asyncness::No => hir::IsAsync::NotAsync,
};
hir::FnHeader {
unsafety: sig.unsafety,
safety: sig.safety,
constness: self.tcx.constness(sig_id),
asyncness,
abi: sig.abi,
Expand Down Expand Up @@ -341,7 +341,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

fn generate_header_error(&self) -> hir::FnHeader {
hir::FnHeader {
unsafety: hir::Unsafety::Normal,
safety: hir::Safety::Default,
constness: hir::Constness::NotConst,
asyncness: hir::IsAsync::NotAsync,
abi: abi::Abi::Rust,
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1360,7 +1360,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::IsAsync::NotAsync
};
hir::FnHeader {
unsafety: self.lower_unsafety(h.unsafety),
safety: self.lower_safety(h.safety),
asyncness: asyncness,
constness: self.lower_constness(h.constness),
abi: self.lower_extern(h.ext),
Expand Down Expand Up @@ -1417,6 +1417,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
}

pub(super) fn lower_safety(&mut self, u: Safety) -> hir::Safety {
match u {
Safety::Unsafe(_) => hir::Safety::Unsafe,
Safety::Default => hir::Safety::Default,
}
}

/// Return the pair of the lowered `generics` as `hir::Generics` and the evaluation of `f` with
/// the carried impl trait definitions and bounds.
#[instrument(level = "debug", skip(self, f))]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1324,7 +1324,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let generic_params = self.lower_lifetime_binder(t.id, &f.generic_params);
hir::TyKind::BareFn(self.arena.alloc(hir::BareFnTy {
generic_params,
unsafety: self.lower_unsafety(f.unsafety),
safety: self.lower_safety(f.safety),
abi: self.lower_extern(f.ext),
decl: self.lower_fn_decl(&f.decl, t.id, t.span, FnDeclKind::Pointer, None),
param_names: self.lower_fn_params_to_names(&f.decl),
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,17 +521,17 @@ impl<'a> AstValidator<'a> {
fn check_foreign_fn_headerless(
&self,
// Deconstruct to ensure exhaustiveness
FnHeader { unsafety, coroutine_kind, constness, ext }: FnHeader,
FnHeader { safety, coroutine_kind, constness, ext }: FnHeader,
) {
let report_err = |span| {
self.dcx().emit_err(errors::FnQualifierInExtern {
span: span,
block: self.current_extern_span(),
});
};
match unsafety {
Unsafe::Yes(span) => report_err(span),
Unsafe::No => (),
match safety {
Safety::Unsafe(span) => report_err(span),
Safety::Default => (),
}
match coroutine_kind {
Some(knd) => report_err(knd.span()),
Expand Down Expand Up @@ -592,7 +592,7 @@ impl<'a> AstValidator<'a> {
(Some(FnCtxt::Free), Some(header)) => match header.ext {
Extern::Explicit(StrLit { symbol_unescaped: sym::C, .. }, _)
| Extern::Implicit(_)
if matches!(header.unsafety, Unsafe::Yes(_)) =>
if matches!(header.safety, Safety::Unsafe(_)) =>
{
return;
}
Expand Down
15 changes: 11 additions & 4 deletions compiler/rustc_ast_pretty/src/pprust/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,7 @@ impl<'a> State<'a> {
self.pclose();
}
ast::TyKind::BareFn(f) => {
self.print_ty_fn(f.ext, f.unsafety, &f.decl, None, &f.generic_params);
self.print_ty_fn(f.ext, f.safety, &f.decl, None, &f.generic_params);
}
ast::TyKind::Path(None, path) => {
self.print_path(path, false, 0);
Expand Down Expand Up @@ -1908,7 +1908,7 @@ impl<'a> State<'a> {
fn print_ty_fn(
&mut self,
ext: ast::Extern,
unsafety: ast::Unsafe,
safety: ast::Safety,
decl: &ast::FnDecl,
name: Option<Ident>,
generic_params: &[ast::GenericParam],
Expand All @@ -1924,15 +1924,15 @@ impl<'a> State<'a> {
},
span: DUMMY_SP,
};
let header = ast::FnHeader { unsafety, ext, ..ast::FnHeader::default() };
let header = ast::FnHeader { safety, ext, ..ast::FnHeader::default() };
self.print_fn(decl, header, name, &generics);
self.end();
}

fn print_fn_header_info(&mut self, header: ast::FnHeader) {
self.print_constness(header.constness);
header.coroutine_kind.map(|coroutine_kind| self.print_coroutine_kind(coroutine_kind));
self.print_unsafety(header.unsafety);
self.print_safety(header.safety);

match header.ext {
ast::Extern::None => {}
Expand All @@ -1956,6 +1956,13 @@ impl<'a> State<'a> {
}
}

fn print_safety(&mut self, s: ast::Safety) {
match s {
ast::Safety::Default => {}
ast::Safety::Unsafe(_) => self.word_nbsp("unsafe"),
}
}

fn print_constness(&mut self, s: ast::Const) {
match s {
ast::Const::No => {}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
liberated_sig.inputs().iter().copied(),
peeled_ty,
liberated_sig.c_variadic,
hir::Unsafety::Normal,
hir::Safety::Default,
rustc_target::spec::abi::Abi::Rust,
)),
);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/type_check/input_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
user_provided_sig.inputs().iter().copied(),
output_ty,
user_provided_sig.c_variadic,
user_provided_sig.unsafety,
user_provided_sig.safety,
user_provided_sig.abi,
);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2007,13 +2007,13 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}

CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(unsafety)) => {
CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(safety)) => {
let sig = match op.ty(body, tcx).kind() {
ty::Closure(_, args) => args.as_closure().sig(),
_ => bug!(),
};
let ty_fn_ptr_from =
Ty::new_fn_ptr(tcx, tcx.signature_unclosure(sig, *unsafety));
Ty::new_fn_ptr(tcx, tcx.signature_unclosure(sig, *safety));

if let Err(terr) = self.eq_types(
*ty,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_builtin_macros/src/alloc_error_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::util::check_builtin_macro_attribute;

use rustc_ast::ptr::P;
use rustc_ast::{self as ast, FnHeader, FnSig, Generics, StmtKind};
use rustc_ast::{Fn, ItemKind, Stmt, TyKind, Unsafe};
use rustc_ast::{Fn, ItemKind, Safety, Stmt, TyKind};
use rustc_expand::base::{Annotatable, ExtCtxt};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::Span;
Expand Down Expand Up @@ -78,7 +78,7 @@ fn generate_handler(cx: &ExtCtxt<'_>, handler: Ident, span: Span, sig_span: Span
let never = ast::FnRetTy::Ty(cx.ty(span, TyKind::Never));
let params = thin_vec![cx.param(span, size, ty_usize.clone()), cx.param(span, align, ty_usize)];
let decl = cx.fn_decl(params, never);
let header = FnHeader { unsafety: Unsafe::Yes(span), ..FnHeader::default() };
let header = FnHeader { safety: Safety::Unsafe(span), ..FnHeader::default() };
let sig = FnSig { decl, header, span: span };

let body = Some(cx.block_expr(call));
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_builtin_macros/src/global_allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_ast::expand::allocator::{
};
use rustc_ast::ptr::P;
use rustc_ast::{self as ast, AttrVec, Expr, FnHeader, FnSig, Generics, Param, StmtKind};
use rustc_ast::{Fn, ItemKind, Mutability, Stmt, Ty, TyKind, Unsafe};
use rustc_ast::{Fn, ItemKind, Mutability, Safety, Stmt, Ty, TyKind};
use rustc_expand::base::{Annotatable, ExtCtxt};
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::Span;
Expand Down Expand Up @@ -73,7 +73,7 @@ impl AllocFnFactory<'_, '_> {
let result = self.call_allocator(method.name, args);
let output_ty = self.ret_ty(&method.output);
let decl = self.cx.fn_decl(abi_args, ast::FnRetTy::Ty(output_ty));
let header = FnHeader { unsafety: Unsafe::Yes(self.span), ..FnHeader::default() };
let header = FnHeader { safety: Safety::Unsafe(self.span), ..FnHeader::default() };
let sig = FnSig { decl, header, span: self.span };
let body = Some(self.cx.block_expr(result));
let kind = ItemKind::Fn(Box::new(Fn {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ fn check_test_signature(
let has_should_panic_attr = attr::contains_name(&i.attrs, sym::should_panic);
let dcx = cx.dcx();

if let ast::Unsafe::Yes(span) = f.sig.header.unsafety {
if let ast::Safety::Unsafe(span) = f.sig.header.safety {
return Err(dcx.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "unsafe" }));
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_cranelift/src/value_and_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ pub(crate) fn assert_assignable<'tcx>(
let FnSig {
inputs_and_output: types_from,
c_variadic: c_variadic_from,
unsafety: unsafety_from,
safety: unsafety_from,
abi: abi_from,
} = from_sig;
let to_sig = fx
Expand All @@ -881,7 +881,7 @@ pub(crate) fn assert_assignable<'tcx>(
let FnSig {
inputs_and_output: types_to,
c_variadic: c_variadic_to,
unsafety: unsafety_to,
safety: unsafety_to,
abi: abi_to,
} = to_sig;
let mut types_from = types_from.iter();
Expand Down
12 changes: 4 additions & 8 deletions compiler/rustc_codegen_gcc/src/intrinsic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,11 +670,7 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {
let step3 = self.or(left, right);

// Fourth step.
if width == 8 {
step3
} else {
self.gcc_bswap(step3, width)
}
if width == 8 { step3 } else { self.gcc_bswap(step3, width) }
}
128 => {
// TODO(antoyo): find a more efficient implementation?
Expand Down Expand Up @@ -1225,7 +1221,7 @@ fn get_rust_try_fn<'a, 'gcc, 'tcx>(
iter::once(i8p),
tcx.types.unit,
false,
rustc_hir::Unsafety::Unsafe,
rustc_hir::Safety::Unsafe,
Abi::Rust,
)),
);
Expand All @@ -1236,7 +1232,7 @@ fn get_rust_try_fn<'a, 'gcc, 'tcx>(
[i8p, i8p].iter().cloned(),
tcx.types.unit,
false,
rustc_hir::Unsafety::Unsafe,
rustc_hir::Safety::Unsafe,
Abi::Rust,
)),
);
Expand All @@ -1245,7 +1241,7 @@ fn get_rust_try_fn<'a, 'gcc, 'tcx>(
[try_fn_ty, i8p, catch_fn_ty],
tcx.types.i32,
false,
rustc_hir::Unsafety::Unsafe,
rustc_hir::Safety::Unsafe,
Abi::Rust,
));
let rust_try = gen_fn(cx, "__rust_try", rust_fn_sig, codegen);
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,7 @@ fn get_rust_try_fn<'ll, 'tcx>(
[i8p],
tcx.types.unit,
false,
hir::Unsafety::Unsafe,
hir::Safety::Unsafe,
Abi::Rust,
)),
);
Expand All @@ -1001,7 +1001,7 @@ fn get_rust_try_fn<'ll, 'tcx>(
[i8p, i8p],
tcx.types.unit,
false,
hir::Unsafety::Unsafe,
hir::Safety::Unsafe,
Abi::Rust,
)),
);
Expand All @@ -1010,7 +1010,7 @@ fn get_rust_try_fn<'ll, 'tcx>(
[try_fn_ty, i8p, catch_fn_ty],
tcx.types.i32,
false,
hir::Unsafety::Unsafe,
hir::Safety::Unsafe,
Abi::Rust,
));
let rust_try = gen_fn(cx, "__rust_try", rust_fn_sig, codegen);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
sym::target_feature => {
if !tcx.is_closure_like(did.to_def_id())
&& let Some(fn_sig) = fn_sig()
&& fn_sig.skip_binder().unsafety() == hir::Unsafety::Normal
&& fn_sig.skip_binder().safety() == hir::Safety::Default
{
if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
// The `#[target_feature]` attribute is allowed on
Expand Down
Loading
Loading