From c90b6b8d2937cc116c635aa5c58f90afcc535248 Mon Sep 17 00:00:00 2001 From: Skgland Date: Thu, 9 May 2024 15:41:15 +0200 Subject: [PATCH 01/27] stabilize `const_int_from_str` --- library/core/src/lib.rs | 1 - library/core/src/num/error.rs | 2 +- library/core/src/num/mod.rs | 6 ++++-- library/core/tests/lib.rs | 1 - tests/ui/consts/const-eval/parse_ints.rs | 2 -- tests/ui/consts/const-eval/parse_ints.stderr | 4 ++-- 6 files changed, 7 insertions(+), 9 deletions(-) diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index c5a1fca667bc3..ef90333596fdb 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -128,7 +128,6 @@ #![feature(const_hash)] #![feature(const_heap)] #![feature(const_index_range_slice_index)] -#![feature(const_int_from_str)] #![feature(const_intrinsic_copy)] #![feature(const_intrinsic_forget)] #![feature(const_ipv4)] diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index a2d7e6f7b0754..b8e22a8aef955 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -113,7 +113,7 @@ pub enum IntErrorKind { impl ParseIntError { /// Outputs the detailed cause of parsing an integer failing. #[must_use] - #[rustc_const_unstable(feature = "const_int_from_str", issue = "59133")] + #[rustc_const_stable(feature = "const_int_from_str", since = "CURRENT_RUSTC_VERSION")] #[stable(feature = "int_error_matching", since = "1.55.0")] pub const fn kind(&self) -> &IntErrorKind { &self.kind diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index 034af6a0d5731..6010f7cee06eb 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -1387,6 +1387,7 @@ from_str_radix_int_impl! { isize i8 i16 i32 i64 i128 usize u8 u16 u32 u64 u128 } #[doc(hidden)] #[inline(always)] #[unstable(issue = "none", feature = "std_internals")] +#[rustc_const_unstable(issue = "none", feature = "const_int_cannot_overflow")] pub const fn can_not_overflow(radix: u32, is_signed_ty: bool, digits: &[u8]) -> bool { radix <= 16 && digits.len() <= mem::size_of::() * 2 - is_signed_ty as usize } @@ -1410,6 +1411,7 @@ const fn from_str_radix_panic(radix: u32) { intrinsics::const_eval_select((radix,), from_str_radix_panic_ct, from_str_radix_panic_rt); } +#[allow_internal_unstable(const_int_cannot_overflow)] macro_rules! from_str_radix { ($($int_ty:ty)+) => {$( impl $int_ty { @@ -1436,7 +1438,7 @@ macro_rules! from_str_radix { #[doc = concat!("assert_eq!(", stringify!($int_ty), "::from_str_radix(\"A\", 16), Ok(10));")] /// ``` #[stable(feature = "rust1", since = "1.0.0")] - #[rustc_const_unstable(feature = "const_int_from_str", issue = "59133")] + #[rustc_const_stable(feature = "const_int_from_str", since = "CURRENT_RUSTC_VERSION")] pub const fn from_str_radix(src: &str, radix: u32) -> Result<$int_ty, ParseIntError> { use self::IntErrorKind::*; use self::ParseIntError as PIE; @@ -1566,7 +1568,7 @@ macro_rules! from_str_radix_size_impl { #[doc = concat!("assert_eq!(", stringify!($size), "::from_str_radix(\"A\", 16), Ok(10));")] /// ``` #[stable(feature = "rust1", since = "1.0.0")] - #[rustc_const_unstable(feature = "const_int_from_str", issue = "59133")] + #[rustc_const_stable(feature = "const_int_from_str", since = "CURRENT_RUSTC_VERSION")] pub const fn from_str_radix(src: &str, radix: u32) -> Result<$size, ParseIntError> { match <$t>::from_str_radix(src, radix) { Ok(x) => Ok(x as $size), diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 83a615fcd8be3..feef5e81480c4 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -16,7 +16,6 @@ #![feature(const_hash)] #![feature(const_heap)] #![feature(const_intrinsic_copy)] -#![feature(const_int_from_str)] #![feature(const_maybe_uninit_as_mut_ptr)] #![feature(const_nonnull_new)] #![feature(const_pointer_is_aligned)] diff --git a/tests/ui/consts/const-eval/parse_ints.rs b/tests/ui/consts/const-eval/parse_ints.rs index ff9fc47e65c33..cb9a3eb431299 100644 --- a/tests/ui/consts/const-eval/parse_ints.rs +++ b/tests/ui/consts/const-eval/parse_ints.rs @@ -1,5 +1,3 @@ -#![feature(const_int_from_str)] - const _OK: () = match i32::from_str_radix("-1234", 10) { Ok(x) => assert!(x == -1234), Err(_) => panic!(), diff --git a/tests/ui/consts/const-eval/parse_ints.stderr b/tests/ui/consts/const-eval/parse_ints.stderr index 9e49fe433a126..ec9249ece8e39 100644 --- a/tests/ui/consts/const-eval/parse_ints.stderr +++ b/tests/ui/consts/const-eval/parse_ints.stderr @@ -6,7 +6,7 @@ error[E0080]: evaluation of constant value failed note: inside `core::num::::from_str_radix` --> $SRC_DIR/core/src/num/mod.rs:LL:COL note: inside `_TOO_LOW` - --> $DIR/parse_ints.rs:7:24 + --> $DIR/parse_ints.rs:5:24 | LL | const _TOO_LOW: () = { u64::from_str_radix("12345ABCD", 1); }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -20,7 +20,7 @@ error[E0080]: evaluation of constant value failed note: inside `core::num::::from_str_radix` --> $SRC_DIR/core/src/num/mod.rs:LL:COL note: inside `_TOO_HIGH` - --> $DIR/parse_ints.rs:8:25 + --> $DIR/parse_ints.rs:6:25 | LL | const _TOO_HIGH: () = { u64::from_str_radix("12345ABCD", 37); }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From eb799cf634a811d1e0d719d30cba83d5611f87c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Thu, 4 Jul 2024 20:51:50 +0200 Subject: [PATCH 02/27] mark `can_not_overflow` as `#[rustc_const_stable(...)]` see https://github.com/rust-lang/rust/pull/124941#discussion_r1664676739 --- library/core/src/num/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index 6010f7cee06eb..0522365e22e0a 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -1387,7 +1387,7 @@ from_str_radix_int_impl! { isize i8 i16 i32 i64 i128 usize u8 u16 u32 u64 u128 } #[doc(hidden)] #[inline(always)] #[unstable(issue = "none", feature = "std_internals")] -#[rustc_const_unstable(issue = "none", feature = "const_int_cannot_overflow")] +#[rustc_const_stable(feature = "const_int_from_str", since = "CURRENT_RUSTC_VERSION")] pub const fn can_not_overflow(radix: u32, is_signed_ty: bool, digits: &[u8]) -> bool { radix <= 16 && digits.len() <= mem::size_of::() * 2 - is_signed_ty as usize } @@ -1411,7 +1411,6 @@ const fn from_str_radix_panic(radix: u32) { intrinsics::const_eval_select((radix,), from_str_radix_panic_ct, from_str_radix_panic_rt); } -#[allow_internal_unstable(const_int_cannot_overflow)] macro_rules! from_str_radix { ($($int_ty:ty)+) => {$( impl $int_ty { From f99df29d9dcc1c313593967c8c390fae39d462d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Thu, 4 Jul 2024 21:23:55 +0200 Subject: [PATCH 03/27] fix tests after rebase --- src/tools/clippy/tests/ui/from_str_radix_10.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/clippy/tests/ui/from_str_radix_10.rs b/src/tools/clippy/tests/ui/from_str_radix_10.rs index 2d5b351f8da3e..0df6a0a202ae7 100644 --- a/src/tools/clippy/tests/ui/from_str_radix_10.rs +++ b/src/tools/clippy/tests/ui/from_str_radix_10.rs @@ -1,4 +1,3 @@ -#![feature(const_int_from_str)] #![warn(clippy::from_str_radix_10)] mod some_mod { @@ -61,7 +60,8 @@ fn main() -> Result<(), Box> { Ok(()) } -fn issue_12732() { +// https://github.com/rust-lang/rust-clippy/issues/12731 +fn issue_12731() { const A: Result = u32::from_str_radix("123", 10); const B: () = { let _ = u32::from_str_radix("123", 10); From 404519a6e5a2e95d0b8f057e35a7d6280fb9c562 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Thu, 4 Jul 2024 22:31:53 +0200 Subject: [PATCH 04/27] bless tests --- .../clippy/tests/ui/from_str_radix_10.fixed | 4 ++-- .../clippy/tests/ui/from_str_radix_10.stderr | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/tools/clippy/tests/ui/from_str_radix_10.fixed b/src/tools/clippy/tests/ui/from_str_radix_10.fixed index f9ce1defda17c..6c582190b4424 100644 --- a/src/tools/clippy/tests/ui/from_str_radix_10.fixed +++ b/src/tools/clippy/tests/ui/from_str_radix_10.fixed @@ -1,4 +1,3 @@ -#![feature(const_int_from_str)] #![warn(clippy::from_str_radix_10)] mod some_mod { @@ -61,7 +60,8 @@ fn main() -> Result<(), Box> { Ok(()) } -fn issue_12732() { +// https://github.com/rust-lang/rust-clippy/issues/12731 +fn issue_12731() { const A: Result = u32::from_str_radix("123", 10); const B: () = { let _ = u32::from_str_radix("123", 10); diff --git a/src/tools/clippy/tests/ui/from_str_radix_10.stderr b/src/tools/clippy/tests/ui/from_str_radix_10.stderr index 01a1bf8940a12..4aa84eca26120 100644 --- a/src/tools/clippy/tests/ui/from_str_radix_10.stderr +++ b/src/tools/clippy/tests/ui/from_str_radix_10.stderr @@ -1,5 +1,5 @@ error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> tests/ui/from_str_radix_10.rs:29:5 + --> tests/ui/from_str_radix_10.rs:28:5 | LL | u32::from_str_radix("30", 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"30".parse::()` @@ -8,43 +8,43 @@ LL | u32::from_str_radix("30", 10)?; = help: to override `-D warnings` add `#[allow(clippy::from_str_radix_10)]` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> tests/ui/from_str_radix_10.rs:32:5 + --> tests/ui/from_str_radix_10.rs:31:5 | LL | i64::from_str_radix("24", 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"24".parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> tests/ui/from_str_radix_10.rs:34:5 + --> tests/ui/from_str_radix_10.rs:33:5 | LL | isize::from_str_radix("100", 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"100".parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> tests/ui/from_str_radix_10.rs:36:5 + --> tests/ui/from_str_radix_10.rs:35:5 | LL | u8::from_str_radix("7", 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"7".parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> tests/ui/from_str_radix_10.rs:38:5 + --> tests/ui/from_str_radix_10.rs:37:5 | LL | u16::from_str_radix(&("10".to_owned() + "5"), 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("10".to_owned() + "5").parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> tests/ui/from_str_radix_10.rs:40:5 + --> tests/ui/from_str_radix_10.rs:39:5 | LL | i128::from_str_radix(Test + Test, 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(Test + Test).parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> tests/ui/from_str_radix_10.rs:44:5 + --> tests/ui/from_str_radix_10.rs:43:5 | LL | i32::from_str_radix(string, 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> tests/ui/from_str_radix_10.rs:48:5 + --> tests/ui/from_str_radix_10.rs:47:5 | LL | i32::from_str_radix(&stringier, 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `stringier.parse::()` From ceec6ddf9e2d4a41828fce4587180029588ae8eb Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 16 Jul 2024 22:06:34 +0200 Subject: [PATCH 05/27] update text for E0736 and E0739 --- .../src/error_codes/E0736.md | 20 ++++++++++++++----- .../src/error_codes/E0739.md | 2 +- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0736.md b/compiler/rustc_error_codes/src/error_codes/E0736.md index 0f3d41ba66dc4..08aa85e705d99 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0736.md +++ b/compiler/rustc_error_codes/src/error_codes/E0736.md @@ -1,14 +1,24 @@ -`#[track_caller]` and `#[naked]` cannot both be applied to the same function. +Functions marked with the `#[naked]` attribute are restricted in what other +code generation attributes they may be marked with. + +The following code generation attributes are incompatible with `#[naked]`: + + * `#[inline]` + * `#[track_caller]` + * `#[target_feature]` Erroneous code example: ```compile_fail,E0736 +#[inline] #[naked] -#[track_caller] fn foo() {} ``` -This is primarily due to ABI incompatibilities between the two attributes. -See [RFC 2091] for details on this and other limitations. +These incompatibilities are due to the fact that naked functions deliberately +impose strict restrictions regarding the code that the compiler is +allowed to produce for this function. + +See [the reference page for codegen attributes] for more information. -[RFC 2091]: https://github.com/rust-lang/rfcs/blob/master/text/2091-inline-semantic.md +[the reference page for codegen attributes]: https://doc.rust-lang.org/reference/attributes/codegen.html diff --git a/compiler/rustc_error_codes/src/error_codes/E0739.md b/compiler/rustc_error_codes/src/error_codes/E0739.md index 8d9039bef93f6..406d3d52779db 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0739.md +++ b/compiler/rustc_error_codes/src/error_codes/E0739.md @@ -1,4 +1,4 @@ -`#[track_caller]` can not be applied on struct. +`#[track_caller]` must be applied to a function Erroneous code example: From 4bd36324b6a19afdbcb53de1d41e65b0060a6dbb Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 16 Jul 2024 23:03:13 +0200 Subject: [PATCH 06/27] improve error message when `#[naked]` is used with `#[inline]` --- compiler/rustc_passes/messages.ftl | 11 +++-- compiler/rustc_passes/src/check_attr.rs | 33 +++++++++++---- compiler/rustc_passes/src/errors.rs | 24 +++++------ compiler/rustc_passes/src/naked_functions.rs | 14 +------ tests/ui/asm/naked-functions-inline.rs | 31 ++++++++++++++ tests/ui/asm/naked-functions-inline.stderr | 27 ++++++++++++ tests/ui/asm/naked-functions.rs | 31 -------------- tests/ui/asm/naked-functions.stderr | 44 ++------------------ 8 files changed, 104 insertions(+), 111 deletions(-) create mode 100644 tests/ui/asm/naked-functions-inline.rs create mode 100644 tests/ui/asm/naked-functions-inline.stderr diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 1d93cbaddd6fe..2b6841181a7ff 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -69,9 +69,6 @@ passes_break_non_loop = .suggestion = use `break` on its own without a value inside this `{$kind}` loop .break_expr_suggestion = alternatively, you might have meant to use the available loop label -passes_cannot_inline_naked_function = - naked functions cannot be inlined - passes_cannot_stabilize_deprecated = an API can't be stabilized after it is deprecated .label = invalid version @@ -485,6 +482,11 @@ passes_naked_functions_asm_block = passes_naked_functions_asm_options = asm options unsupported in naked functions: {$unsupported_options} +passes_naked_functions_codegen_attribute = + cannot use additional code generation attributes with `#[naked]` + .label = this attribute is incompatible with `#[naked]` + .label2 = function marked with `#[naked]` here + passes_naked_functions_must_use_noreturn = asm in naked functions must use `noreturn` option .suggestion = consider specifying that the asm block is responsible for returning from the function @@ -492,9 +494,6 @@ passes_naked_functions_must_use_noreturn = passes_naked_functions_operands = only `const` and `sym` operands are supported in naked functions -passes_naked_tracked_caller = - cannot use `#[track_caller]` with `#[naked]` - passes_no_link = attribute should be applied to an `extern crate` item .label = not an `extern crate` item diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index ce2fa83810fe8..739847d73d31c 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -155,7 +155,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { [sym::rustc_std_internal_symbol] => { self.check_rustc_std_internal_symbol(attr, span, target) } - [sym::naked] => self.check_naked(hir_id, attr, span, target), + [sym::naked] => self.check_naked(hir_id, attr, span, target, attrs), [sym::rustc_never_returns_null_ptr] => { self.check_applied_to_fn_or_method(hir_id, attr, span, target) } @@ -410,12 +410,33 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } /// Checks if `#[naked]` is applied to a function definition. - fn check_naked(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) -> bool { + fn check_naked( + &self, + hir_id: HirId, + attr: &Attribute, + span: Span, + target: Target, + attrs: &[Attribute], + ) -> bool { + const FORBIDDEN: [rustc_span::Symbol; 3] = + [sym::track_caller, sym::inline, sym::target_feature]; + + for other_attr in attrs { + if FORBIDDEN.into_iter().any(|name| other_attr.has_name(name)) { + self.dcx().emit_err(errors::NakedFunctionCodegenAttribute { + span: other_attr.span, + naked_span: attr.span, + }); + + return false; + } + } + match target { Target::Fn | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true, // FIXME(#80564): We permit struct fields, match arms and macro defs to have an - // `#[allow_internal_unstable]` attribute with just a lint, because we previously + // `#[naked]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible // with crates depending on them, we can't throw an error here. Target::Field | Target::Arm | Target::MacroDef => { @@ -488,7 +509,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } } - /// Checks if a `#[track_caller]` is applied to a non-naked function. Returns `true` if valid. + /// Checks if a `#[track_caller]` is applied to a function. Returns `true` if valid. fn check_track_caller( &self, hir_id: HirId, @@ -498,10 +519,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { target: Target, ) -> bool { match target { - _ if attrs.iter().any(|attr| attr.has_name(sym::naked)) => { - self.dcx().emit_err(errors::NakedTrackedCaller { attr_span }); - false - } Target::Fn => { // `#[track_caller]` is not valid on weak lang items because they are called via // `extern` declarations and `#[track_caller]` would alter their ABI. diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 58d27d5b4bbaa..03105795bfcd9 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -79,13 +79,6 @@ pub struct AttrShouldBeAppliedToFn { pub on_crate: bool, } -#[derive(Diagnostic)] -#[diag(passes_naked_tracked_caller, code = E0736)] -pub struct NakedTrackedCaller { - #[primary_span] - pub attr_span: Span, -} - #[derive(Diagnostic)] #[diag(passes_should_be_applied_to_fn, code = E0739)] pub struct TrackedCallerWrongLocation { @@ -1124,13 +1117,6 @@ pub struct UnlabeledCfInWhileCondition<'a> { pub cf_type: &'a str, } -#[derive(Diagnostic)] -#[diag(passes_cannot_inline_naked_function)] -pub struct CannotInlineNakedFunction { - #[primary_span] - pub span: Span, -} - #[derive(LintDiagnostic)] #[diag(passes_undefined_naked_function_abi)] pub struct UndefinedNakedFunctionAbi; @@ -1196,6 +1182,16 @@ pub struct NakedFunctionsMustUseNoreturn { pub last_span: Span, } +#[derive(Diagnostic)] +#[diag(passes_naked_functions_codegen_attribute, code = E0736)] +pub struct NakedFunctionCodegenAttribute { + #[primary_span] + #[label] + pub span: Span, + #[label(passes_label2)] + pub naked_span: Span, +} + #[derive(Diagnostic)] #[diag(passes_attr_only_in_functions)] pub struct AttrOnlyInFunctions { diff --git a/compiler/rustc_passes/src/naked_functions.rs b/compiler/rustc_passes/src/naked_functions.rs index d45ee32a624de..4040fbd182ede 100644 --- a/compiler/rustc_passes/src/naked_functions.rs +++ b/compiler/rustc_passes/src/naked_functions.rs @@ -14,9 +14,8 @@ use rustc_span::Span; use rustc_target::spec::abi::Abi; use crate::errors::{ - CannotInlineNakedFunction, NakedFunctionsAsmBlock, NakedFunctionsAsmOptions, - NakedFunctionsMustUseNoreturn, NakedFunctionsOperands, NoPatterns, ParamsNotAllowed, - UndefinedNakedFunctionAbi, + NakedFunctionsAsmBlock, NakedFunctionsAsmOptions, NakedFunctionsMustUseNoreturn, + NakedFunctionsOperands, NoPatterns, ParamsNotAllowed, UndefinedNakedFunctionAbi, }; pub(crate) fn provide(providers: &mut Providers) { @@ -53,15 +52,6 @@ fn check_mod_naked_functions(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) { check_no_patterns(tcx, body.params); check_no_parameters_use(tcx, body); check_asm(tcx, def_id, body); - check_inline(tcx, def_id); - } -} - -/// Check that the function isn't inlined. -fn check_inline(tcx: TyCtxt<'_>, def_id: LocalDefId) { - let attrs = tcx.get_attrs(def_id, sym::inline); - for attr in attrs { - tcx.dcx().emit_err(CannotInlineNakedFunction { span: attr.span }); } } diff --git a/tests/ui/asm/naked-functions-inline.rs b/tests/ui/asm/naked-functions-inline.rs new file mode 100644 index 0000000000000..9a4f754751842 --- /dev/null +++ b/tests/ui/asm/naked-functions-inline.rs @@ -0,0 +1,31 @@ +//@ needs-asm-support +#![feature(naked_functions)] +#![crate_type = "lib"] + +use std::arch::asm; + +#[naked] +pub unsafe extern "C" fn inline_none() { + asm!("", options(noreturn)); +} + +#[naked] +#[inline] +//~^ ERROR [E0736] +pub unsafe extern "C" fn inline_hint() { + asm!("", options(noreturn)); +} + +#[naked] +#[inline(always)] +//~^ ERROR [E0736] +pub unsafe extern "C" fn inline_always() { + asm!("", options(noreturn)); +} + +#[naked] +#[inline(never)] +//~^ ERROR [E0736] +pub unsafe extern "C" fn inline_never() { + asm!("", options(noreturn)); +} diff --git a/tests/ui/asm/naked-functions-inline.stderr b/tests/ui/asm/naked-functions-inline.stderr new file mode 100644 index 0000000000000..2496e942b17c8 --- /dev/null +++ b/tests/ui/asm/naked-functions-inline.stderr @@ -0,0 +1,27 @@ +error[E0736]: cannot use additional code generation attributes with `#[naked]` + --> $DIR/naked-functions-inline.rs:13:1 + | +LL | #[naked] + | -------- function marked with `#[naked]` here +LL | #[inline] + | ^^^^^^^^^ this attribute is incompatible with `#[naked]` + +error[E0736]: cannot use additional code generation attributes with `#[naked]` + --> $DIR/naked-functions-inline.rs:20:1 + | +LL | #[naked] + | -------- function marked with `#[naked]` here +LL | #[inline(always)] + | ^^^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` + +error[E0736]: cannot use additional code generation attributes with `#[naked]` + --> $DIR/naked-functions-inline.rs:27:1 + | +LL | #[naked] + | -------- function marked with `#[naked]` here +LL | #[inline(never)] + | ^^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0736`. diff --git a/tests/ui/asm/naked-functions.rs b/tests/ui/asm/naked-functions.rs index 1619ebfcf39f0..e6633ddd4f3e9 100644 --- a/tests/ui/asm/naked-functions.rs +++ b/tests/ui/asm/naked-functions.rs @@ -168,37 +168,6 @@ pub unsafe extern "C" fn inline_none() { } #[naked] -#[inline] -//~^ ERROR naked functions cannot be inlined -pub unsafe extern "C" fn inline_hint() { - asm!("", options(noreturn)); -} - -#[naked] -#[inline(always)] -//~^ ERROR naked functions cannot be inlined -pub unsafe extern "C" fn inline_always() { - asm!("", options(noreturn)); -} - -#[naked] -#[inline(never)] -//~^ ERROR naked functions cannot be inlined -pub unsafe extern "C" fn inline_never() { - asm!("", options(noreturn)); -} - -#[naked] -#[inline] -//~^ ERROR naked functions cannot be inlined -#[inline(always)] -//~^ ERROR naked functions cannot be inlined -#[inline(never)] -//~^ ERROR naked functions cannot be inlined -pub unsafe extern "C" fn inline_all() { - asm!("", options(noreturn)); -} - #[naked] pub unsafe extern "C" fn allow_compile_error(a: u32) -> u32 { compile_error!("this is a user specified error") diff --git a/tests/ui/asm/naked-functions.stderr b/tests/ui/asm/naked-functions.stderr index 77bc80a101f02..736de85765e39 100644 --- a/tests/ui/asm/naked-functions.stderr +++ b/tests/ui/asm/naked-functions.stderr @@ -5,19 +5,19 @@ LL | asm!("", options(readonly, nostack), options(pure)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^ error: this is a user specified error - --> $DIR/naked-functions.rs:204:5 + --> $DIR/naked-functions.rs:173:5 | LL | compile_error!("this is a user specified error") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this is a user specified error - --> $DIR/naked-functions.rs:210:5 + --> $DIR/naked-functions.rs:179:5 | LL | compile_error!("this is a user specified error"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: asm template must be a string literal - --> $DIR/naked-functions.rs:217:10 + --> $DIR/naked-functions.rs:186:10 | LL | asm!(invalid_syntax) | ^^^^^^^^^^^^^^ @@ -249,42 +249,6 @@ warning: Rust ABI is unsupported in naked functions LL | pub unsafe fn rust_abi() { | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: naked functions cannot be inlined - --> $DIR/naked-functions.rs:171:1 - | -LL | #[inline] - | ^^^^^^^^^ - -error: naked functions cannot be inlined - --> $DIR/naked-functions.rs:178:1 - | -LL | #[inline(always)] - | ^^^^^^^^^^^^^^^^^ - -error: naked functions cannot be inlined - --> $DIR/naked-functions.rs:185:1 - | -LL | #[inline(never)] - | ^^^^^^^^^^^^^^^^ - -error: naked functions cannot be inlined - --> $DIR/naked-functions.rs:192:1 - | -LL | #[inline] - | ^^^^^^^^^ - -error: naked functions cannot be inlined - --> $DIR/naked-functions.rs:194:1 - | -LL | #[inline(always)] - | ^^^^^^^^^^^^^^^^^ - -error: naked functions cannot be inlined - --> $DIR/naked-functions.rs:196:1 - | -LL | #[inline(never)] - | ^^^^^^^^^^^^^^^^ - -error: aborting due to 33 previous errors; 2 warnings emitted +error: aborting due to 27 previous errors; 2 warnings emitted For more information about this error, try `rustc --explain E0787`. From 7e6c083873b7b98aa52d47896107af11560aeaf5 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 16 Jul 2024 23:35:02 +0200 Subject: [PATCH 07/27] improve error message when `#[naked]` is used with `#[track-caller] and `#[target-feature]`` --- compiler/rustc_passes/messages.ftl | 2 +- compiler/rustc_passes/src/check_attr.rs | 26 ++++++++++--------- compiler/rustc_passes/src/errors.rs | 2 +- .../ui/asm/naked-functions-target-feature.rs | 13 ++++++++++ .../asm/naked-functions-target-feature.stderr | 12 +++++++++ .../rfc-2091-track-caller/error-with-naked.rs | 4 +-- .../error-with-naked.stderr | 14 +++++++--- 7 files changed, 53 insertions(+), 20 deletions(-) create mode 100644 tests/ui/asm/naked-functions-target-feature.rs create mode 100644 tests/ui/asm/naked-functions-target-feature.stderr diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 2b6841181a7ff..8dec24feebaeb 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -485,7 +485,7 @@ passes_naked_functions_asm_options = passes_naked_functions_codegen_attribute = cannot use additional code generation attributes with `#[naked]` .label = this attribute is incompatible with `#[naked]` - .label2 = function marked with `#[naked]` here + .naked_attribute = function marked with `#[naked]` here passes_naked_functions_must_use_noreturn = asm in naked functions must use `noreturn` option diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 739847d73d31c..311c11a03886e 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -421,20 +421,22 @@ impl<'tcx> CheckAttrVisitor<'tcx> { const FORBIDDEN: [rustc_span::Symbol; 3] = [sym::track_caller, sym::inline, sym::target_feature]; - for other_attr in attrs { - if FORBIDDEN.into_iter().any(|name| other_attr.has_name(name)) { - self.dcx().emit_err(errors::NakedFunctionCodegenAttribute { - span: other_attr.span, - naked_span: attr.span, - }); - - return false; - } - } - match target { Target::Fn - | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true, + | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => { + for other_attr in attrs { + if FORBIDDEN.into_iter().any(|name| other_attr.has_name(name)) { + self.dcx().emit_err(errors::NakedFunctionCodegenAttribute { + span: other_attr.span, + naked_span: attr.span, + }); + + return false; + } + } + + true + } // FIXME(#80564): We permit struct fields, match arms and macro defs to have an // `#[naked]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 03105795bfcd9..b6be096b43b12 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1188,7 +1188,7 @@ pub struct NakedFunctionCodegenAttribute { #[primary_span] #[label] pub span: Span, - #[label(passes_label2)] + #[label(passes_naked_attribute)] pub naked_span: Span, } diff --git a/tests/ui/asm/naked-functions-target-feature.rs b/tests/ui/asm/naked-functions-target-feature.rs new file mode 100644 index 0000000000000..264e7e0976b1d --- /dev/null +++ b/tests/ui/asm/naked-functions-target-feature.rs @@ -0,0 +1,13 @@ +//@ only-x86_64 +//@ needs-asm-support +#![feature(naked_functions)] +#![crate_type = "lib"] + +use std::arch::asm; + +#[target_feature(enable = "sse2")] +//~^ ERROR [E0736] +#[naked] +pub unsafe extern "C" fn naked_target_feature() { + asm!("", options(noreturn)); +} diff --git a/tests/ui/asm/naked-functions-target-feature.stderr b/tests/ui/asm/naked-functions-target-feature.stderr new file mode 100644 index 0000000000000..1a8b59f61fe0a --- /dev/null +++ b/tests/ui/asm/naked-functions-target-feature.stderr @@ -0,0 +1,12 @@ +error[E0736]: cannot use additional code generation attributes with `#[naked]` + --> $DIR/naked-functions-target-feature.rs:8:1 + | +LL | #[target_feature(enable = "sse2")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` +LL | +LL | #[naked] + | -------- function marked with `#[naked]` here + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0736`. diff --git a/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.rs b/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.rs index 6eaa7d4d9bc9f..0c73b9abf351d 100644 --- a/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.rs +++ b/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.rs @@ -3,7 +3,7 @@ use std::arch::asm; -#[track_caller] //~ ERROR cannot use `#[track_caller]` with `#[naked]` +#[track_caller] //~ ERROR [E0736] //~^ ERROR `#[track_caller]` requires Rust ABI #[naked] extern "C" fn f() { @@ -15,7 +15,7 @@ extern "C" fn f() { struct S; impl S { - #[track_caller] //~ ERROR cannot use `#[track_caller]` with `#[naked]` + #[track_caller] //~ ERROR [E0736] //~^ ERROR `#[track_caller]` requires Rust ABI #[naked] extern "C" fn g() { diff --git a/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr b/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr index 04c5c649d7fd8..7ab9f8b39ebda 100644 --- a/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr +++ b/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr @@ -1,14 +1,20 @@ -error[E0736]: cannot use `#[track_caller]` with `#[naked]` +error[E0736]: cannot use additional code generation attributes with `#[naked]` --> $DIR/error-with-naked.rs:6:1 | LL | #[track_caller] - | ^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` +LL | +LL | #[naked] + | -------- function marked with `#[naked]` here -error[E0736]: cannot use `#[track_caller]` with `#[naked]` +error[E0736]: cannot use additional code generation attributes with `#[naked]` --> $DIR/error-with-naked.rs:18:5 | LL | #[track_caller] - | ^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` +LL | +LL | #[naked] + | -------- function marked with `#[naked]` here error[E0737]: `#[track_caller]` requires Rust ABI --> $DIR/error-with-naked.rs:6:1 From 4d082b77af1f714df6c407785e1961a9dddd554c Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 17 Jul 2024 00:03:33 +0200 Subject: [PATCH 08/27] add error message when `#[naked]` is used with `#[test]` --- compiler/rustc_builtin_macros/messages.ftl | 5 +++ compiler/rustc_builtin_macros/src/errors.rs | 10 +++++ compiler/rustc_builtin_macros/src/test.rs | 8 ++++ .../src/error_codes/E0798.md | 14 +++++++ compiler/rustc_error_codes/src/lib.rs | 1 + tests/ui/asm/naked-functions-testattrs.rs | 39 +++++++++++++++++++ tests/ui/asm/naked-functions-testattrs.stderr | 35 +++++++++++++++++ 7 files changed, 112 insertions(+) create mode 100644 compiler/rustc_error_codes/src/error_codes/E0798.md create mode 100644 tests/ui/asm/naked-functions-testattrs.rs create mode 100644 tests/ui/asm/naked-functions-testattrs.stderr diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl index b56bfa98357b3..21c0f0802f7c8 100644 --- a/compiler/rustc_builtin_macros/messages.ftl +++ b/compiler/rustc_builtin_macros/messages.ftl @@ -216,6 +216,11 @@ builtin_macros_multiple_defaults = multiple declared defaults .note = only one variant can be default .suggestion = make `{$ident}` default +builtin_macros_naked_functions_testing_attribute = + cannot use `#[naked]` with testing attributes + .label = function marked with testing attribute here + .naked_attribute = `#[naked]` is incompatible with testing attributes + builtin_macros_no_default_variant = no default declared .help = make a unit variant default by placing `#[default]` above it .suggestion = make `{$ident}` default diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index 49d640436c2f3..24706a3b05445 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/compiler/rustc_builtin_macros/src/errors.rs @@ -912,3 +912,13 @@ pub(crate) struct ExpectedItem<'a> { pub span: Span, pub token: &'a str, } + +#[derive(Diagnostic)] +#[diag(builtin_macros_naked_functions_testing_attribute, code = E0798)] +pub struct NakedFunctionTestingAttribute { + #[primary_span] + #[label(builtin_macros_naked_attribute)] + pub naked_span: Span, + #[label] + pub testing_span: Span, +} diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs index c0310a2f4b003..bb00c8de1b808 100644 --- a/compiler/rustc_builtin_macros/src/test.rs +++ b/compiler/rustc_builtin_macros/src/test.rs @@ -133,6 +133,14 @@ pub(crate) fn expand_test_or_bench( }; }; + if let Some(attr) = attr::find_by_name(&item.attrs, sym::naked) { + cx.dcx().emit_err(errors::NakedFunctionTestingAttribute { + testing_span: attr_sp, + naked_span: attr.span, + }); + return vec![Annotatable::Item(item)]; + } + // check_*_signature will report any errors in the type so compilation // will fail. We shouldn't try to expand in this case because the errors // would be spurious. diff --git a/compiler/rustc_error_codes/src/error_codes/E0798.md b/compiler/rustc_error_codes/src/error_codes/E0798.md new file mode 100644 index 0000000000000..96f25eb4f0e2c --- /dev/null +++ b/compiler/rustc_error_codes/src/error_codes/E0798.md @@ -0,0 +1,14 @@ +Testing attributes cannot be applied to functions marked with `#[naked]`. + +Erroneous code example: + +```ignore (requires test runner) +#[test] +#[should_panic] +#[naked] +fn foo() {} +``` + +See [the reference page for testing attributes] for more information. + +[the reference page for testing attributes]: https://doc.rust-lang.org/reference/attributes/testing.html diff --git a/compiler/rustc_error_codes/src/lib.rs b/compiler/rustc_error_codes/src/lib.rs index d13d5e1bca219..2a7bc2501c081 100644 --- a/compiler/rustc_error_codes/src/lib.rs +++ b/compiler/rustc_error_codes/src/lib.rs @@ -536,6 +536,7 @@ E0794: 0794, E0795: 0795, E0796: 0796, E0797: 0797, +E0798: 0798, ); ) } diff --git a/tests/ui/asm/naked-functions-testattrs.rs b/tests/ui/asm/naked-functions-testattrs.rs new file mode 100644 index 0000000000000..593cf3ad4595c --- /dev/null +++ b/tests/ui/asm/naked-functions-testattrs.rs @@ -0,0 +1,39 @@ +//@ needs-asm-support +//@ compile-flags: --test + +#![allow(undefined_naked_function_abi)] +#![feature(naked_functions)] +#![feature(test)] +#![crate_type = "lib"] + +use std::arch::asm; + +#[test] +#[naked] +//~^ ERROR [E0798] +fn test_naked() { + unsafe { asm!("", options(noreturn)) }; +} + +#[should_panic] +#[test] +#[naked] +//~^ ERROR [E0798] +fn test_naked_should_panic() { + unsafe { asm!("", options(noreturn)) }; +} + +#[ignore] +#[test] +#[naked] +//~^ ERROR [E0798] +fn test_naked_ignore() { + unsafe { asm!("", options(noreturn)) }; +} + +#[bench] +#[naked] +//~^ ERROR [E0798] +fn bench_naked() { + unsafe { asm!("", options(noreturn)) }; +} diff --git a/tests/ui/asm/naked-functions-testattrs.stderr b/tests/ui/asm/naked-functions-testattrs.stderr new file mode 100644 index 0000000000000..87647b0ca37df --- /dev/null +++ b/tests/ui/asm/naked-functions-testattrs.stderr @@ -0,0 +1,35 @@ +error[E0798]: cannot use `#[naked]` with testing attributes + --> $DIR/naked-functions-testattrs.rs:12:1 + | +LL | #[test] + | ------- function marked with testing attribute here +LL | #[naked] + | ^^^^^^^^ `#[naked]` is incompatible with testing attributes + +error[E0798]: cannot use `#[naked]` with testing attributes + --> $DIR/naked-functions-testattrs.rs:20:1 + | +LL | #[test] + | ------- function marked with testing attribute here +LL | #[naked] + | ^^^^^^^^ `#[naked]` is incompatible with testing attributes + +error[E0798]: cannot use `#[naked]` with testing attributes + --> $DIR/naked-functions-testattrs.rs:28:1 + | +LL | #[test] + | ------- function marked with testing attribute here +LL | #[naked] + | ^^^^^^^^ `#[naked]` is incompatible with testing attributes + +error[E0798]: cannot use `#[naked]` with testing attributes + --> $DIR/naked-functions-testattrs.rs:35:1 + | +LL | #[bench] + | -------- function marked with testing attribute here +LL | #[naked] + | ^^^^^^^^ `#[naked]` is incompatible with testing attributes + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0798`. From 4596ce6dd586e7c11216ad16b258c86b73651338 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 17 Jul 2024 10:42:24 +0200 Subject: [PATCH 09/27] switch to an allowlist approach --- .../src/error_codes/E0736.md | 15 ++--- compiler/rustc_passes/messages.ftl | 4 +- compiler/rustc_passes/src/check_attr.rs | 40 +++++++++++-- compiler/rustc_passes/src/errors.rs | 4 +- tests/ui/asm/naked-functions-inline.stderr | 6 +- .../asm/naked-functions-target-feature.stderr | 2 +- tests/ui/asm/naked-functions.rs | 60 +++++++++++++++++-- tests/ui/asm/naked-functions.stderr | 6 +- .../error-with-naked.stderr | 4 +- 9 files changed, 110 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0736.md b/compiler/rustc_error_codes/src/error_codes/E0736.md index 08aa85e705d99..4660d6107448d 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0736.md +++ b/compiler/rustc_error_codes/src/error_codes/E0736.md @@ -1,11 +1,12 @@ Functions marked with the `#[naked]` attribute are restricted in what other -code generation attributes they may be marked with. +attributes they may be marked with. -The following code generation attributes are incompatible with `#[naked]`: +Notable attributes that are incompatible with `#[naked]` are: - * `#[inline]` - * `#[track_caller]` - * `#[target_feature]` +* `#[inline]` +* `#[track_caller]` +* `#[target_feature]` +* `#[test]`, `#[ignore]`, `#[should_panic]` Erroneous code example: @@ -18,7 +19,3 @@ fn foo() {} These incompatibilities are due to the fact that naked functions deliberately impose strict restrictions regarding the code that the compiler is allowed to produce for this function. - -See [the reference page for codegen attributes] for more information. - -[the reference page for codegen attributes]: https://doc.rust-lang.org/reference/attributes/codegen.html diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 8dec24feebaeb..4638ab62379e6 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -482,8 +482,8 @@ passes_naked_functions_asm_block = passes_naked_functions_asm_options = asm options unsupported in naked functions: {$unsupported_options} -passes_naked_functions_codegen_attribute = - cannot use additional code generation attributes with `#[naked]` +passes_naked_functions_incompatible_attribute = + attribute incompatible with `#[naked]` .label = this attribute is incompatible with `#[naked]` .naked_attribute = function marked with `#[naked]` here diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 311c11a03886e..0d5ba2af846f9 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -418,15 +418,47 @@ impl<'tcx> CheckAttrVisitor<'tcx> { target: Target, attrs: &[Attribute], ) -> bool { - const FORBIDDEN: [rustc_span::Symbol; 3] = - [sym::track_caller, sym::inline, sym::target_feature]; + // many attributes don't make sense in combination with #[naked]. + // Notable attributes that are incompatible with `#[naked]` are: + // + // * `#[inline]` + // * `#[track_caller]` + // * `#[target_feature]` + // * `#[test]`, `#[ignore]`, `#[should_panic]` + // + // NOTE: when making changes to this list, check that `error_codes/E0736.md` remains accurate + const ALLOW_LIST: &[rustc_span::Symbol] = &[ + // conditional compilation + sym::cfg, + sym::cfg_attr, + // testing (allowed here so better errors can be generated in `rustc_builtin_macros::test`) + sym::test, + sym::ignore, + sym::should_panic, + // diagnostics + sym::allow, + sym::warn, + sym::deny, + sym::forbid, + sym::deprecated, + sym::must_use, + // abi, linking and FFI + sym::export_name, + sym::link_section, + sym::no_mangle, + sym::naked, + // code generation + sym::cold, + // documentation + sym::doc, + ]; match target { Target::Fn | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => { for other_attr in attrs { - if FORBIDDEN.into_iter().any(|name| other_attr.has_name(name)) { - self.dcx().emit_err(errors::NakedFunctionCodegenAttribute { + if !ALLOW_LIST.iter().any(|name| other_attr.has_name(*name)) { + self.dcx().emit_err(errors::NakedFunctionIncompatibleAttribute { span: other_attr.span, naked_span: attr.span, }); diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index b6be096b43b12..7b41ad3eb37e5 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1183,8 +1183,8 @@ pub struct NakedFunctionsMustUseNoreturn { } #[derive(Diagnostic)] -#[diag(passes_naked_functions_codegen_attribute, code = E0736)] -pub struct NakedFunctionCodegenAttribute { +#[diag(passes_naked_functions_incompatible_attribute, code = E0736)] +pub struct NakedFunctionIncompatibleAttribute { #[primary_span] #[label] pub span: Span, diff --git a/tests/ui/asm/naked-functions-inline.stderr b/tests/ui/asm/naked-functions-inline.stderr index 2496e942b17c8..9754551b90a88 100644 --- a/tests/ui/asm/naked-functions-inline.stderr +++ b/tests/ui/asm/naked-functions-inline.stderr @@ -1,4 +1,4 @@ -error[E0736]: cannot use additional code generation attributes with `#[naked]` +error[E0736]: attribute incompatible with `#[naked]` --> $DIR/naked-functions-inline.rs:13:1 | LL | #[naked] @@ -6,7 +6,7 @@ LL | #[naked] LL | #[inline] | ^^^^^^^^^ this attribute is incompatible with `#[naked]` -error[E0736]: cannot use additional code generation attributes with `#[naked]` +error[E0736]: attribute incompatible with `#[naked]` --> $DIR/naked-functions-inline.rs:20:1 | LL | #[naked] @@ -14,7 +14,7 @@ LL | #[naked] LL | #[inline(always)] | ^^^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` -error[E0736]: cannot use additional code generation attributes with `#[naked]` +error[E0736]: attribute incompatible with `#[naked]` --> $DIR/naked-functions-inline.rs:27:1 | LL | #[naked] diff --git a/tests/ui/asm/naked-functions-target-feature.stderr b/tests/ui/asm/naked-functions-target-feature.stderr index 1a8b59f61fe0a..f233449476113 100644 --- a/tests/ui/asm/naked-functions-target-feature.stderr +++ b/tests/ui/asm/naked-functions-target-feature.stderr @@ -1,4 +1,4 @@ -error[E0736]: cannot use additional code generation attributes with `#[naked]` +error[E0736]: attribute incompatible with `#[naked]` --> $DIR/naked-functions-target-feature.rs:8:1 | LL | #[target_feature(enable = "sse2")] diff --git a/tests/ui/asm/naked-functions.rs b/tests/ui/asm/naked-functions.rs index e6633ddd4f3e9..ab11468189ebe 100644 --- a/tests/ui/asm/naked-functions.rs +++ b/tests/ui/asm/naked-functions.rs @@ -162,11 +162,6 @@ pub unsafe extern "C" fn valid_att_syntax() { asm!("", options(noreturn, att_syntax)); } -#[naked] -pub unsafe extern "C" fn inline_none() { - asm!("", options(noreturn)); -} - #[naked] #[naked] pub unsafe extern "C" fn allow_compile_error(a: u32) -> u32 { @@ -186,3 +181,58 @@ pub unsafe extern "C" fn invalid_asm_syntax(a: u32) -> u32 { asm!(invalid_syntax) //~^ ERROR asm template must be a string literal } + +#[cfg(target_arch = "x86_64")] +#[cfg_attr(target_pointer_width = "64", no_mangle)] +#[naked] +pub unsafe extern "C" fn compatible_cfg_attributes() { + asm!("", options(noreturn, att_syntax)); +} + +#[allow(dead_code)] +#[warn(dead_code)] +#[deny(dead_code)] +#[forbid(dead_code)] +#[naked] +pub unsafe extern "C" fn compatible_diagnostic_attributes() { + asm!("", options(noreturn, att_syntax)); +} + +#[deprecated = "test"] +#[naked] +pub unsafe extern "C" fn compatible_deprecated_attributes() { + asm!("", options(noreturn, att_syntax)); +} + +#[cfg(target_arch = "x86_64")] +#[must_use] +#[naked] +pub unsafe extern "C" fn compatible_must_use_attributes() -> u64 { + asm!( + " + mov rax, 42 + ret + ", + options(noreturn) + ) +} + +#[export_name = "exported_function_name"] +#[link_section = ".custom_section"] +#[no_mangle] +#[naked] +pub unsafe extern "C" fn compatible_ffi_attributes_1() { + asm!("", options(noreturn, att_syntax)); +} + +#[cold] +#[naked] +pub unsafe extern "C" fn compatible_codegen_attributes() { + asm!("", options(noreturn, att_syntax)); +} + +#[doc = "foo bar baz"] +#[naked] +pub unsafe extern "C" fn compatible_doc_attributes() { + asm!("", options(noreturn, att_syntax)); +} diff --git a/tests/ui/asm/naked-functions.stderr b/tests/ui/asm/naked-functions.stderr index 736de85765e39..357c365479467 100644 --- a/tests/ui/asm/naked-functions.stderr +++ b/tests/ui/asm/naked-functions.stderr @@ -5,19 +5,19 @@ LL | asm!("", options(readonly, nostack), options(pure)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^ error: this is a user specified error - --> $DIR/naked-functions.rs:173:5 + --> $DIR/naked-functions.rs:168:5 | LL | compile_error!("this is a user specified error") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this is a user specified error - --> $DIR/naked-functions.rs:179:5 + --> $DIR/naked-functions.rs:174:5 | LL | compile_error!("this is a user specified error"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: asm template must be a string literal - --> $DIR/naked-functions.rs:186:10 + --> $DIR/naked-functions.rs:181:10 | LL | asm!(invalid_syntax) | ^^^^^^^^^^^^^^ diff --git a/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr b/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr index 7ab9f8b39ebda..1f2eb06aa577d 100644 --- a/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr +++ b/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr @@ -1,4 +1,4 @@ -error[E0736]: cannot use additional code generation attributes with `#[naked]` +error[E0736]: attribute incompatible with `#[naked]` --> $DIR/error-with-naked.rs:6:1 | LL | #[track_caller] @@ -7,7 +7,7 @@ LL | LL | #[naked] | -------- function marked with `#[naked]` here -error[E0736]: cannot use additional code generation attributes with `#[naked]` +error[E0736]: attribute incompatible with `#[naked]` --> $DIR/error-with-naked.rs:18:5 | LL | #[track_caller] From 0ed639fe486c8471ee8febf53c305454646200d6 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 17 Jul 2024 10:50:13 +0200 Subject: [PATCH 10/27] merge error codes --- compiler/rustc_builtin_macros/src/errors.rs | 2 +- .../rustc_error_codes/src/error_codes/E0798.md | 14 -------------- compiler/rustc_error_codes/src/lib.rs | 1 - tests/ui/asm/naked-functions-testattrs.rs | 8 ++++---- tests/ui/asm/naked-functions-testattrs.stderr | 10 +++++----- 5 files changed, 10 insertions(+), 25 deletions(-) delete mode 100644 compiler/rustc_error_codes/src/error_codes/E0798.md diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index 24706a3b05445..daf40df46c308 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/compiler/rustc_builtin_macros/src/errors.rs @@ -914,7 +914,7 @@ pub(crate) struct ExpectedItem<'a> { } #[derive(Diagnostic)] -#[diag(builtin_macros_naked_functions_testing_attribute, code = E0798)] +#[diag(builtin_macros_naked_functions_testing_attribute, code = E0736)] pub struct NakedFunctionTestingAttribute { #[primary_span] #[label(builtin_macros_naked_attribute)] diff --git a/compiler/rustc_error_codes/src/error_codes/E0798.md b/compiler/rustc_error_codes/src/error_codes/E0798.md deleted file mode 100644 index 96f25eb4f0e2c..0000000000000 --- a/compiler/rustc_error_codes/src/error_codes/E0798.md +++ /dev/null @@ -1,14 +0,0 @@ -Testing attributes cannot be applied to functions marked with `#[naked]`. - -Erroneous code example: - -```ignore (requires test runner) -#[test] -#[should_panic] -#[naked] -fn foo() {} -``` - -See [the reference page for testing attributes] for more information. - -[the reference page for testing attributes]: https://doc.rust-lang.org/reference/attributes/testing.html diff --git a/compiler/rustc_error_codes/src/lib.rs b/compiler/rustc_error_codes/src/lib.rs index 2a7bc2501c081..d13d5e1bca219 100644 --- a/compiler/rustc_error_codes/src/lib.rs +++ b/compiler/rustc_error_codes/src/lib.rs @@ -536,7 +536,6 @@ E0794: 0794, E0795: 0795, E0796: 0796, E0797: 0797, -E0798: 0798, ); ) } diff --git a/tests/ui/asm/naked-functions-testattrs.rs b/tests/ui/asm/naked-functions-testattrs.rs index 593cf3ad4595c..12943ac0378b7 100644 --- a/tests/ui/asm/naked-functions-testattrs.rs +++ b/tests/ui/asm/naked-functions-testattrs.rs @@ -10,7 +10,7 @@ use std::arch::asm; #[test] #[naked] -//~^ ERROR [E0798] +//~^ ERROR [E0736] fn test_naked() { unsafe { asm!("", options(noreturn)) }; } @@ -18,7 +18,7 @@ fn test_naked() { #[should_panic] #[test] #[naked] -//~^ ERROR [E0798] +//~^ ERROR [E0736] fn test_naked_should_panic() { unsafe { asm!("", options(noreturn)) }; } @@ -26,14 +26,14 @@ fn test_naked_should_panic() { #[ignore] #[test] #[naked] -//~^ ERROR [E0798] +//~^ ERROR [E0736] fn test_naked_ignore() { unsafe { asm!("", options(noreturn)) }; } #[bench] #[naked] -//~^ ERROR [E0798] +//~^ ERROR [E0736] fn bench_naked() { unsafe { asm!("", options(noreturn)) }; } diff --git a/tests/ui/asm/naked-functions-testattrs.stderr b/tests/ui/asm/naked-functions-testattrs.stderr index 87647b0ca37df..4dabe41964a57 100644 --- a/tests/ui/asm/naked-functions-testattrs.stderr +++ b/tests/ui/asm/naked-functions-testattrs.stderr @@ -1,4 +1,4 @@ -error[E0798]: cannot use `#[naked]` with testing attributes +error[E0736]: cannot use `#[naked]` with testing attributes --> $DIR/naked-functions-testattrs.rs:12:1 | LL | #[test] @@ -6,7 +6,7 @@ LL | #[test] LL | #[naked] | ^^^^^^^^ `#[naked]` is incompatible with testing attributes -error[E0798]: cannot use `#[naked]` with testing attributes +error[E0736]: cannot use `#[naked]` with testing attributes --> $DIR/naked-functions-testattrs.rs:20:1 | LL | #[test] @@ -14,7 +14,7 @@ LL | #[test] LL | #[naked] | ^^^^^^^^ `#[naked]` is incompatible with testing attributes -error[E0798]: cannot use `#[naked]` with testing attributes +error[E0736]: cannot use `#[naked]` with testing attributes --> $DIR/naked-functions-testattrs.rs:28:1 | LL | #[test] @@ -22,7 +22,7 @@ LL | #[test] LL | #[naked] | ^^^^^^^^ `#[naked]` is incompatible with testing attributes -error[E0798]: cannot use `#[naked]` with testing attributes +error[E0736]: cannot use `#[naked]` with testing attributes --> $DIR/naked-functions-testattrs.rs:35:1 | LL | #[bench] @@ -32,4 +32,4 @@ LL | #[naked] error: aborting due to 4 previous errors -For more information about this error, try `rustc --explain E0798`. +For more information about this error, try `rustc --explain E0736`. From 6f2318c53cd80d7c7bc3077d890c6fafce4942c0 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Jul 2024 16:02:32 +0200 Subject: [PATCH 11/27] allow `#[target_feature]` on `#[naked]` functions --- compiler/rustc_error_codes/src/error_codes/E0736.md | 1 - compiler/rustc_passes/src/check_attr.rs | 2 +- tests/ui/asm/naked-functions-target-feature.rs | 13 ------------- tests/ui/asm/naked-functions-target-feature.stderr | 12 ------------ tests/ui/asm/naked-functions.rs | 6 ++++++ 5 files changed, 7 insertions(+), 27 deletions(-) delete mode 100644 tests/ui/asm/naked-functions-target-feature.rs delete mode 100644 tests/ui/asm/naked-functions-target-feature.stderr diff --git a/compiler/rustc_error_codes/src/error_codes/E0736.md b/compiler/rustc_error_codes/src/error_codes/E0736.md index 4660d6107448d..cb7633b7068a3 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0736.md +++ b/compiler/rustc_error_codes/src/error_codes/E0736.md @@ -5,7 +5,6 @@ Notable attributes that are incompatible with `#[naked]` are: * `#[inline]` * `#[track_caller]` -* `#[target_feature]` * `#[test]`, `#[ignore]`, `#[should_panic]` Erroneous code example: diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 0d5ba2af846f9..f5cb674646684 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -423,7 +423,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { // // * `#[inline]` // * `#[track_caller]` - // * `#[target_feature]` // * `#[test]`, `#[ignore]`, `#[should_panic]` // // NOTE: when making changes to this list, check that `error_codes/E0736.md` remains accurate @@ -449,6 +448,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { sym::naked, // code generation sym::cold, + sym::target_feature, // documentation sym::doc, ]; diff --git a/tests/ui/asm/naked-functions-target-feature.rs b/tests/ui/asm/naked-functions-target-feature.rs deleted file mode 100644 index 264e7e0976b1d..0000000000000 --- a/tests/ui/asm/naked-functions-target-feature.rs +++ /dev/null @@ -1,13 +0,0 @@ -//@ only-x86_64 -//@ needs-asm-support -#![feature(naked_functions)] -#![crate_type = "lib"] - -use std::arch::asm; - -#[target_feature(enable = "sse2")] -//~^ ERROR [E0736] -#[naked] -pub unsafe extern "C" fn naked_target_feature() { - asm!("", options(noreturn)); -} diff --git a/tests/ui/asm/naked-functions-target-feature.stderr b/tests/ui/asm/naked-functions-target-feature.stderr deleted file mode 100644 index f233449476113..0000000000000 --- a/tests/ui/asm/naked-functions-target-feature.stderr +++ /dev/null @@ -1,12 +0,0 @@ -error[E0736]: attribute incompatible with `#[naked]` - --> $DIR/naked-functions-target-feature.rs:8:1 - | -LL | #[target_feature(enable = "sse2")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` -LL | -LL | #[naked] - | -------- function marked with `#[naked]` here - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0736`. diff --git a/tests/ui/asm/naked-functions.rs b/tests/ui/asm/naked-functions.rs index ab11468189ebe..18363252b0b8b 100644 --- a/tests/ui/asm/naked-functions.rs +++ b/tests/ui/asm/naked-functions.rs @@ -231,6 +231,12 @@ pub unsafe extern "C" fn compatible_codegen_attributes() { asm!("", options(noreturn, att_syntax)); } +#[target_feature(enable = "sse2")] +#[naked] +pub unsafe extern "C" fn compatible_target_feature() { + asm!("", options(noreturn)); +} + #[doc = "foo bar baz"] #[naked] pub unsafe extern "C" fn compatible_doc_attributes() { From 370fcce5646a55a15f2753bb3d8f249d065e192e Mon Sep 17 00:00:00 2001 From: binarycat Date: Thu, 25 Jul 2024 17:51:35 -0400 Subject: [PATCH 12/27] rustdoc: change title of search results the current title is too similar to that of the page for std::result::Result, which is a problem both for navigating to the Result docs via browser autocomplete, and for being able to tell which tab is which when the width of tabs is small. --- src/librustdoc/html/static/js/search.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 86af38f3ee75e..c6618c9859c79 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -2932,7 +2932,7 @@ ${item.displayPath}${name}\ } // Update document title to maintain a meaningful browser history - searchState.title = "Results for " + query.original + " - Rust"; + searchState.title = '"' + query.original + '" Search - Rust'; // Because searching is incremental by character, only the most // recent search query is added to the browser history. From 587b64e88b19d6e5ca0fed53f3538168df949ed5 Mon Sep 17 00:00:00 2001 From: binarycat Date: Thu, 25 Jul 2024 18:55:45 -0400 Subject: [PATCH 13/27] use double quotes --- src/librustdoc/html/static/js/search.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index c6618c9859c79..3eb27ea087c1d 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -2932,7 +2932,7 @@ ${item.displayPath}${name}\ } // Update document title to maintain a meaningful browser history - searchState.title = '"' + query.original + '" Search - Rust'; + searchState.title = "\"" + query.original + "\" Search - Rust"; // Because searching is incremental by character, only the most // recent search query is added to the browser history. From caee195bdd922c25946276625993b93a5bc0eb41 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 08:42:43 +1000 Subject: [PATCH 14/27] Invert early exit conditions in `collect_tokens_trailing_token`. This has been bugging me for a while. I find complex "if any of these are true" conditions easier to think about than complex "if all of these are true" conditions, because you can stop as soon as one is true. --- .../rustc_parse/src/parser/attr_wrapper.rs | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index dc5f98f7be8b6..6413aa092414e 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -199,20 +199,20 @@ impl<'a> Parser<'a> { force_collect: ForceCollect, f: impl FnOnce(&mut Self, ast::AttrVec) -> PResult<'a, (R, bool)>, ) -> PResult<'a, R> { - // Skip collection when nothing could observe the collected tokens, i.e. - // all of the following conditions hold. - // - We are not force collecting tokens (because force collection - // requires tokens by definition). - if matches!(force_collect, ForceCollect::No) - // - None of our outer attributes require tokens. - && attrs.is_complete() - // - Our target doesn't support custom inner attributes (custom + // We must collect if anything could observe the collected tokens, i.e. + // if any of the following conditions hold. + // - We are force collecting tokens (because force collection requires + // tokens by definition). + let needs_collection = matches!(force_collect, ForceCollect::Yes) + // - Any of our outer attributes require tokens. + || !attrs.is_complete() + // - Our target supports custom inner attributes (custom // inner attribute invocation might require token capturing). - && !R::SUPPORTS_CUSTOM_INNER_ATTRS - // - We are not in `capture_cfg` mode (which requires tokens if + || R::SUPPORTS_CUSTOM_INNER_ATTRS + // - We are in `capture_cfg` mode (which requires tokens if // the parsed node has `#[cfg]` or `#[cfg_attr]` attributes). - && !self.capture_cfg - { + || self.capture_cfg; + if !needs_collection { return Ok(f(self, attrs.attrs)?.0); } @@ -250,28 +250,28 @@ impl<'a> Parser<'a> { return Ok(ret); } - // This is similar to the "skip collection" check at the start of this - // function, but now that we've parsed an AST node we have more + // This is similar to the `needs_collection` check at the start of this + // function, but now that we've parsed an AST node we have complete // information available. (If we return early here that means the // setup, such as cloning the token cursor, was unnecessary. That's // hard to avoid.) // - // Skip collection when nothing could observe the collected tokens, i.e. - // all of the following conditions hold. - // - We are not force collecting tokens. - if matches!(force_collect, ForceCollect::No) - // - None of our outer *or* inner attributes require tokens. + // We must collect if anything could observe the collected tokens, i.e. + // if any of the following conditions hold. + // - We are force collecting tokens. + let needs_collection = matches!(force_collect, ForceCollect::Yes) + // - Any of our outer *or* inner attributes require tokens. // (`attrs` was just outer attributes, but `ret.attrs()` is outer - // and inner attributes. That makes this check more precise than - // `attrs.is_complete()` at the start of the function, and we can - // skip the subsequent check on `R::SUPPORTS_CUSTOM_INNER_ATTRS`. - && crate::parser::attr::is_complete(ret.attrs()) - // - We are not in `capture_cfg` mode, or we are but there are no - // `#[cfg]` or `#[cfg_attr]` attributes. (During normal - // non-`capture_cfg` parsing, we don't need any special capturing - // for those attributes, because they're builtin.) - && (!self.capture_cfg || !has_cfg_or_cfg_attr(ret.attrs())) - { + // and inner attributes. So this check is more precise than the + // earlier `attrs.is_complete()` check, and we don't need to + // check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.) + || !crate::parser::attr::is_complete(ret.attrs()) + // - We are in `capture_cfg` mode and there are `#[cfg]` or + // `#[cfg_attr]` attributes. (During normal non-`capture_cfg` + // parsing, we don't need any special capturing for those + // attributes, because they're builtin.) + || (self.capture_cfg && has_cfg_or_cfg_attr(ret.attrs())); + if !needs_collection { return Ok(ret); } From 4288edb219fb288af524b490bd3830fc7cfafe02 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 09:00:17 +1000 Subject: [PATCH 15/27] Inline and remove `AttrWrapper::is_complete`. It has a single call site. This change makes the two `needs_collect` conditions more similar to each other, and therefore easier to understand. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 6413aa092414e..27b899300e7d9 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -60,10 +60,6 @@ impl AttrWrapper { pub fn is_empty(&self) -> bool { self.attrs.is_empty() } - - pub fn is_complete(&self) -> bool { - crate::parser::attr::is_complete(&self.attrs) - } } /// Returns `true` if `attrs` contains a `cfg` or `cfg_attr` attribute @@ -205,7 +201,7 @@ impl<'a> Parser<'a> { // tokens by definition). let needs_collection = matches!(force_collect, ForceCollect::Yes) // - Any of our outer attributes require tokens. - || !attrs.is_complete() + || !crate::parser::attr::is_complete(&attrs.attrs) // - Our target supports custom inner attributes (custom // inner attribute invocation might require token capturing). || R::SUPPORTS_CUSTOM_INNER_ATTRS @@ -261,9 +257,9 @@ impl<'a> Parser<'a> { // - We are force collecting tokens. let needs_collection = matches!(force_collect, ForceCollect::Yes) // - Any of our outer *or* inner attributes require tokens. - // (`attrs` was just outer attributes, but `ret.attrs()` is outer - // and inner attributes. So this check is more precise than the - // earlier `attrs.is_complete()` check, and we don't need to + // (`attr.attrs` was just outer attributes, but `ret.attrs()` is + // outer and inner attributes. So this check is more precise than + // the earlier `is_complete()` check, and we don't need to // check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.) || !crate::parser::attr::is_complete(ret.attrs()) // - We are in `capture_cfg` mode and there are `#[cfg]` or From 3d363c3d99a5e27fa0b604aae4ca745b4cb4c43a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 09:03:30 +1000 Subject: [PATCH 16/27] Move `is_complete` to the module that uses it. And make it non-`pub`. --- compiler/rustc_parse/src/parser/attr.rs | 11 ----------- compiler/rustc_parse/src/parser/attr_wrapper.rs | 15 +++++++++++++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index 0b2c304403941..535b53a836e98 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -457,14 +457,3 @@ impl<'a> Parser<'a> { Err(self.dcx().create_err(err)) } } - -/// The attributes are complete if all attributes are either a doc comment or a -/// builtin attribute other than `cfg_attr`. -pub fn is_complete(attrs: &[ast::Attribute]) -> bool { - attrs.iter().all(|attr| { - attr.is_doc_comment() - || attr.ident().is_some_and(|ident| { - ident.name != sym::cfg_attr && rustc_feature::is_builtin_attr_name(ident.name) - }) - }) -} diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 27b899300e7d9..82fd2257481ed 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -201,7 +201,7 @@ impl<'a> Parser<'a> { // tokens by definition). let needs_collection = matches!(force_collect, ForceCollect::Yes) // - Any of our outer attributes require tokens. - || !crate::parser::attr::is_complete(&attrs.attrs) + || !is_complete(&attrs.attrs) // - Our target supports custom inner attributes (custom // inner attribute invocation might require token capturing). || R::SUPPORTS_CUSTOM_INNER_ATTRS @@ -261,7 +261,7 @@ impl<'a> Parser<'a> { // outer and inner attributes. So this check is more precise than // the earlier `is_complete()` check, and we don't need to // check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.) - || !crate::parser::attr::is_complete(ret.attrs()) + || !is_complete(ret.attrs()) // - We are in `capture_cfg` mode and there are `#[cfg]` or // `#[cfg_attr]` attributes. (During normal non-`capture_cfg` // parsing, we don't need any special capturing for those @@ -457,6 +457,17 @@ fn make_attr_token_stream( AttrTokenStream::new(stack_top.inner) } +/// The attributes are complete if all attributes are either a doc comment or a +/// builtin attribute other than `cfg_attr`. +fn is_complete(attrs: &[ast::Attribute]) -> bool { + attrs.iter().all(|attr| { + attr.is_doc_comment() + || attr.ident().is_some_and(|ident| { + ident.name != sym::cfg_attr && rustc_feature::is_builtin_attr_name(ident.name) + }) + }) +} + // Some types are used a lot. Make sure they don't unintentionally get bigger. #[cfg(target_pointer_width = "64")] mod size_asserts { From e631b1ebfa273fc4703fa2fd60fde281340d4909 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 09:06:19 +1000 Subject: [PATCH 17/27] Invert the sense of `is_complete` and rename it as `needs_tokens`. I have always found `is_complete` an unhelpful name. The new name (and inverted sense) fits in better with the conditions at its call sites. --- .../rustc_parse/src/parser/attr_wrapper.rs | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 82fd2257481ed..1bc41e68cf372 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -201,7 +201,7 @@ impl<'a> Parser<'a> { // tokens by definition). let needs_collection = matches!(force_collect, ForceCollect::Yes) // - Any of our outer attributes require tokens. - || !is_complete(&attrs.attrs) + || needs_tokens(&attrs.attrs) // - Our target supports custom inner attributes (custom // inner attribute invocation might require token capturing). || R::SUPPORTS_CUSTOM_INNER_ATTRS @@ -259,9 +259,9 @@ impl<'a> Parser<'a> { // - Any of our outer *or* inner attributes require tokens. // (`attr.attrs` was just outer attributes, but `ret.attrs()` is // outer and inner attributes. So this check is more precise than - // the earlier `is_complete()` check, and we don't need to + // the earlier `needs_tokens` check, and we don't need to // check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.) - || !is_complete(ret.attrs()) + || needs_tokens(ret.attrs()) // - We are in `capture_cfg` mode and there are `#[cfg]` or // `#[cfg_attr]` attributes. (During normal non-`capture_cfg` // parsing, we don't need any special capturing for those @@ -457,14 +457,16 @@ fn make_attr_token_stream( AttrTokenStream::new(stack_top.inner) } -/// The attributes are complete if all attributes are either a doc comment or a -/// builtin attribute other than `cfg_attr`. -fn is_complete(attrs: &[ast::Attribute]) -> bool { - attrs.iter().all(|attr| { - attr.is_doc_comment() - || attr.ident().is_some_and(|ident| { - ident.name != sym::cfg_attr && rustc_feature::is_builtin_attr_name(ident.name) - }) +/// Tokens are needed if: +/// - any non-single-segment attributes (other than doc comments) are present; or +/// - any `cfg_attr` attributes are present; +/// - any single-segment, non-builtin attributes are present. +fn needs_tokens(attrs: &[ast::Attribute]) -> bool { + attrs.iter().any(|attr| match attr.ident() { + None => !attr.is_doc_comment(), + Some(ident) => { + ident.name == sym::cfg_attr || !rustc_feature::is_builtin_attr_name(ident.name) + } }) } From a560810a69a09452b4cd0c3173b6af98496dba35 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 10:54:56 +1000 Subject: [PATCH 18/27] Don't include inner attribute ranges in `CaptureState`. The current code is this: ``` self.capture_state.replace_ranges.push((start_pos..end_pos, Some(target))); self.capture_state.replace_ranges.extend(inner_attr_replace_ranges); ``` What's not obvious is that every range in `inner_attr_replace_ranges` must be a strict sub-range of `start_pos..end_pos`. Which means, in `LazyAttrTokenStreamImpl::to_attr_token_stream`, they will be done first, and then the `start_pos..end_pos` replacement will just overwrite them. So they aren't needed. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index dc5f98f7be8b6..1fd0654e0aba9 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -337,8 +337,7 @@ impl<'a> Parser<'a> { // When parsing `m`: // - `start_pos..end_pos` is `0..34` (`mod m`, excluding the `#[cfg_eval]` attribute). // - `inner_attr_replace_ranges` is empty. - // - `replace_range_start..replace_ranges_end` has two entries. - // - One to delete the inner attribute (`17..27`), obtained when parsing `g` (see above). + // - `replace_range_start..replace_ranges_end` has one entry. // - One `AttrsTarget` (added below when parsing `g`) to replace all of `g` (`3..33`, // including its outer attribute), with: // - `attrs`: includes the outer and the inner attr. @@ -369,12 +368,10 @@ impl<'a> Parser<'a> { // What is the status here when parsing the example code at the top of this method? // - // When parsing `g`, we add two entries: + // When parsing `g`, we add one entry: // - The `start_pos..end_pos` (`3..33`) entry has a new `AttrsTarget` with: // - `attrs`: includes the outer and the inner attr. // - `tokens`: lazy tokens for `g` (with its inner attr deleted). - // - `inner_attr_replace_ranges` contains the one entry to delete the inner attr's - // tokens (`17..27`). // // When parsing `m`, we do nothing here. @@ -384,7 +381,6 @@ impl<'a> Parser<'a> { let start_pos = if has_outer_attrs { attrs.start_pos } else { start_pos }; let target = AttrsTarget { attrs: ret.attrs().iter().cloned().collect(), tokens }; self.capture_state.replace_ranges.push((start_pos..end_pos, Some(target))); - self.capture_state.replace_ranges.extend(inner_attr_replace_ranges); } else if matches!(self.capture_state.capturing, Capturing::No) { // Only clear the ranges once we've finished capturing entirely, i.e. we've finished // the outermost call to this method. From 6e87858f26bcb9fadbd60f9e8ee24816a55faa80 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 14:02:15 +1000 Subject: [PATCH 19/27] Fix a comment. Imagine you have replace ranges (2..20,X) and (5..15,Y), and these tokens: ``` a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x ``` If we replace (5..15,Y) first, then (2..20,X) we get this sequence ``` a,b,c,d,e,Y,_,_,_,_,_,_,_,_,_,p,q,r,s,t,u,v,w,x a,b,X,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,u,v,w,x ``` which is what we want. If we do it in the other order, we get this: ``` a,b,X,_,_,_,_,_,_,_,_,_,_,_,_,p,q,r,s,t,u,v,w,x a,b,X,_,_,Y,_,_,_,_,_,_,_,_,_,_,_,_,_,_,u,v,w,x ``` which is wrong. So it's true that we need the `.rev()` but the comment is wrong about why. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 1fd0654e0aba9..a90bfb574e2ee 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -137,9 +137,9 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl { // `#[cfg(FALSE)] struct Foo { #[cfg(FALSE)] field: bool }` // // By starting processing from the replace range with the greatest - // start position, we ensure that any replace range which encloses - // another replace range will capture the *replaced* tokens for the inner - // range, not the original tokens. + // start position, we ensure that any (outer) replace range which + // encloses another (inner) replace range will fully overwrite the + // inner range's replacement. for (range, target) in replace_ranges.into_iter().rev() { assert!(!range.is_empty(), "Cannot replace an empty range: {range:?}"); From 6ea2da5a28580dba214b32831d0a6952e2ebce91 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 13:20:27 +1000 Subject: [PATCH 20/27] Tweak a loop. A fully imperative style is easier to read than a half-iterator, half-imperative style. Also, rename `inner_attr` as `attr` because it might be an outer attribute. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index a90bfb574e2ee..abee668cc6cb0 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -297,11 +297,13 @@ impl<'a> Parser<'a> { // with `None`, which means the relevant tokens will be removed. (More // details below.) let mut inner_attr_replace_ranges = Vec::new(); - for inner_attr in ret.attrs().iter().filter(|a| a.style == ast::AttrStyle::Inner) { - if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&inner_attr.id) { - inner_attr_replace_ranges.push((attr_range, None)); - } else { - self.dcx().span_delayed_bug(inner_attr.span, "Missing token range for attribute"); + for attr in ret.attrs() { + if attr.style == ast::AttrStyle::Inner { + if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&attr.id) { + inner_attr_replace_ranges.push((attr_range, None)); + } else { + self.dcx().span_delayed_bug(attr.span, "Missing token range for attribute"); + } } } From 55d37ae711b1a1ab82ef02d0594f92167e18af90 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 14:00:29 +1000 Subject: [PATCH 21/27] Remove an unnecessary block. --- .../rustc_parse/src/parser/attr_wrapper.rs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index abee668cc6cb0..389e4a6267d2e 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -114,17 +114,15 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl { replace_ranges.sort_by_key(|(range, _)| range.start); #[cfg(debug_assertions)] - { - for [(range, tokens), (next_range, next_tokens)] in replace_ranges.array_windows() { - assert!( - range.end <= next_range.start || range.end >= next_range.end, - "Replace ranges should either be disjoint or nested: ({:?}, {:?}) ({:?}, {:?})", - range, - tokens, - next_range, - next_tokens, - ); - } + for [(range, tokens), (next_range, next_tokens)] in replace_ranges.array_windows() { + assert!( + range.end <= next_range.start || range.end >= next_range.end, + "Replace ranges should either be disjoint or nested: ({:?}, {:?}) ({:?}, {:?})", + range, + tokens, + next_range, + next_tokens, + ); } // Process the replace ranges, starting from the highest start From 33b98bf2566d1ac768fe979cb9c5ff5b30c6d9b6 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 26 Jul 2024 10:19:31 +0000 Subject: [PATCH 22/27] Remove redundant option that was just encoding that a slice was empty --- compiler/rustc_ast_lowering/src/item.rs | 6 +++--- compiler/rustc_ast_lowering/src/lib.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index f990b4ba69b3f..f0186e36c42ae 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -172,7 +172,7 @@ impl<'hir> LoweringContext<'_, 'hir> { id: NodeId, hir_id: hir::HirId, ident: &mut Ident, - attrs: Option<&'hir [Attribute]>, + attrs: &'hir [Attribute], vis_span: Span, i: &ItemKind, ) -> hir::ItemKind<'hir> { @@ -488,7 +488,7 @@ impl<'hir> LoweringContext<'_, 'hir> { id: NodeId, vis_span: Span, ident: &mut Ident, - attrs: Option<&'hir [Attribute]>, + attrs: &'hir [Attribute], ) -> hir::ItemKind<'hir> { let path = &tree.prefix; let segments = prefix.segments.iter().chain(path.segments.iter()).cloned().collect(); @@ -566,7 +566,7 @@ impl<'hir> LoweringContext<'_, 'hir> { // `ItemLocalId` and the new owner. (See `lower_node_id`) let kind = this.lower_use_tree(use_tree, &prefix, id, vis_span, &mut ident, attrs); - if let Some(attrs) = attrs { + if !attrs.is_empty() { this.attrs.insert(hir::ItemLocalId::ZERO, attrs); } diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 0f5f4d8023bd1..01ba57e97b28f 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -913,15 +913,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ret } - fn lower_attrs(&mut self, id: HirId, attrs: &[Attribute]) -> Option<&'hir [Attribute]> { + fn lower_attrs(&mut self, id: HirId, attrs: &[Attribute]) -> &'hir [Attribute] { if attrs.is_empty() { - None + &[] } else { debug_assert_eq!(id.owner, self.current_hir_id_owner); let ret = self.arena.alloc_from_iter(attrs.iter().map(|a| self.lower_attr(a))); debug_assert!(!ret.is_empty()); self.attrs.insert(id.local_id, ret); - Some(ret) + ret } } From 114e0dcf254eb88aa66cad76088a46bf47b82b52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 26 Jul 2024 13:30:52 +0200 Subject: [PATCH 23/27] CI: do not respect custom try jobs for unrolled perf builds --- src/ci/github-actions/calculate-job-matrix.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ci/github-actions/calculate-job-matrix.py b/src/ci/github-actions/calculate-job-matrix.py index d03bbda100807..7de6d5fcd5f75 100755 --- a/src/ci/github-actions/calculate-job-matrix.py +++ b/src/ci/github-actions/calculate-job-matrix.py @@ -97,9 +97,15 @@ def find_run_type(ctx: GitHubCtx) -> Optional[WorkflowRunType]: "refs/heads/automation/bors/try" ) + # Unrolled branch from a rollup for testing perf + # This should **not** allow custom try jobs + is_unrolled_perf_build = ctx.ref == "refs/heads/try-perf" + if try_build: - jobs = get_custom_jobs(ctx) - return TryRunType(custom_jobs=jobs) + custom_jobs = [] + if not is_unrolled_perf_build: + custom_jobs = get_custom_jobs(ctx) + return TryRunType(custom_jobs=custom_jobs) if ctx.ref == "refs/heads/auto": return AutoRunType() From 33dd288ef1d28eabb15c15e739496014198329ab Mon Sep 17 00:00:00 2001 From: Tamme Dittrich Date: Fri, 26 Jul 2024 13:59:43 +0200 Subject: [PATCH 24/27] Add a test case for `extern "C" unsafe` to the ui tests --- .../issue-87217-keyword-order/wrong-unsafe-abi.rs | 12 ++++++++++++ .../wrong-unsafe-abi.stderr | 8 ++++++++ 2 files changed, 20 insertions(+) create mode 100644 tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs create mode 100644 tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr diff --git a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs new file mode 100644 index 0000000000000..c97425d8ade88 --- /dev/null +++ b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs @@ -0,0 +1,12 @@ +//@ edition:2018 + +// There is an order to respect for keywords before a function: +// `, const, async, unsafe, extern, ""` +// +// This test ensures the compiler is helpful about them being misplaced. +// Visibilities are tested elsewhere. + +extern "C" unsafe fn test() {} +//~^ ERROR + +fn main() {} diff --git a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr new file mode 100644 index 0000000000000..42c9764cc1e9e --- /dev/null +++ b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr @@ -0,0 +1,8 @@ +error: expected `{`, found keyword `unsafe` + --> $DIR/wrong-unsafe-abi.rs:9:12 + | +LL | extern "C" unsafe fn test() {} + | ^^^^^^ expected `{` + +error: aborting due to 1 previous error + From 3fdc99193e95a29b6fd2228b0acc5f8b4a81feda Mon Sep 17 00:00:00 2001 From: Tamme Dittrich Date: Fri, 26 Jul 2024 15:14:05 +0200 Subject: [PATCH 25/27] Improve error message for `extern "C" unsafe fn()` This was handled correctly already for `extern unsafe fn()`. Co-authored-by: Folkert --- compiler/rustc_parse/src/parser/item.rs | 9 ++++++--- tests/ui/parser/issues/issue-19398.rs | 6 +++++- tests/ui/parser/issues/issue-19398.stderr | 14 +++++++------- .../issue-87217-keyword-order/wrong-unsafe-abi.rs | 6 +++++- .../wrong-unsafe-abi.stderr | 9 +++++++-- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 9aaf4b99243f5..112855e6d1f5a 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -2483,12 +2483,15 @@ impl<'a> Parser<'a> { /// `check_pub` adds additional `pub` to the checks in case users place it /// wrongly, can be used to ensure `pub` never comes after `default`. pub(super) fn check_fn_front_matter(&mut self, check_pub: bool, case: Case) -> bool { + const ALL_QUALS: &[Symbol] = + &[kw::Pub, kw::Gen, kw::Const, kw::Async, kw::Unsafe, kw::Safe, kw::Extern]; + // We use an over-approximation here. // `const const`, `fn const` won't parse, but we're not stepping over other syntax either. // `pub` is added in case users got confused with the ordering like `async pub fn`, // only if it wasn't preceded by `default` as `default pub` is invalid. let quals: &[Symbol] = if check_pub { - &[kw::Pub, kw::Gen, kw::Const, kw::Async, kw::Unsafe, kw::Safe, kw::Extern] + ALL_QUALS } else { &[kw::Gen, kw::Const, kw::Async, kw::Unsafe, kw::Safe, kw::Extern] }; @@ -2518,9 +2521,9 @@ impl<'a> Parser<'a> { || self.check_keyword_case(kw::Extern, case) && self.look_ahead(1, |t| t.can_begin_string_literal()) && (self.look_ahead(2, |t| t.is_keyword_case(kw::Fn, case)) || - // this branch is only for better diagnostic in later, `pub` is not allowed here + // this branch is only for better diagnostics; `pub`, `unsafe`, etc. are not allowed here (self.may_recover() - && self.look_ahead(2, |t| t.is_keyword(kw::Pub)) + && self.look_ahead(2, |t| ALL_QUALS.iter().any(|&kw| t.is_keyword(kw))) && self.look_ahead(3, |t| t.is_keyword_case(kw::Fn, case)))) } diff --git a/tests/ui/parser/issues/issue-19398.rs b/tests/ui/parser/issues/issue-19398.rs index 46eb320a172f2..358f65f1da51b 100644 --- a/tests/ui/parser/issues/issue-19398.rs +++ b/tests/ui/parser/issues/issue-19398.rs @@ -1,6 +1,10 @@ trait T { extern "Rust" unsafe fn foo(); - //~^ ERROR expected `{`, found keyword `unsafe` + //~^ ERROR expected `fn`, found keyword `unsafe` + //~| NOTE expected `fn` + //~| HELP `unsafe` must come before `extern "Rust"` + //~| SUGGESTION unsafe extern "Rust" + //~| NOTE keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern` } fn main() {} diff --git a/tests/ui/parser/issues/issue-19398.stderr b/tests/ui/parser/issues/issue-19398.stderr index 236fac673b6b3..2b97ec50c9172 100644 --- a/tests/ui/parser/issues/issue-19398.stderr +++ b/tests/ui/parser/issues/issue-19398.stderr @@ -1,13 +1,13 @@ -error: expected `{`, found keyword `unsafe` +error: expected `fn`, found keyword `unsafe` --> $DIR/issue-19398.rs:2:19 | -LL | trait T { - | - while parsing this item list starting here LL | extern "Rust" unsafe fn foo(); - | ^^^^^^ expected `{` -LL | -LL | } - | - the item list ends here + | --------------^^^^^^ + | | | + | | expected `fn` + | help: `unsafe` must come before `extern "Rust"`: `unsafe extern "Rust"` + | + = note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern` error: aborting due to 1 previous error diff --git a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs index c97425d8ade88..794ac635c7694 100644 --- a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs +++ b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs @@ -7,6 +7,10 @@ // Visibilities are tested elsewhere. extern "C" unsafe fn test() {} -//~^ ERROR +//~^ ERROR expected `fn`, found keyword `unsafe` +//~| NOTE expected `fn` +//~| HELP `unsafe` must come before `extern "C"` +//~| SUGGESTION unsafe extern "C" +//~| NOTE keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern` fn main() {} diff --git a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr index 42c9764cc1e9e..8ed037869c829 100644 --- a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr +++ b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr @@ -1,8 +1,13 @@ -error: expected `{`, found keyword `unsafe` +error: expected `fn`, found keyword `unsafe` --> $DIR/wrong-unsafe-abi.rs:9:12 | LL | extern "C" unsafe fn test() {} - | ^^^^^^ expected `{` + | -----------^^^^^^ + | | | + | | expected `fn` + | help: `unsafe` must come before `extern "C"`: `unsafe extern "C"` + | + = note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern` error: aborting due to 1 previous error From 130ce490f55b6ca8dbe65c977bae18b04736d3f3 Mon Sep 17 00:00:00 2001 From: harryscholes Date: Fri, 26 Jul 2024 16:09:17 +0100 Subject: [PATCH 26/27] Fix docs --- library/core/src/iter/traits/iterator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index 733d414d44465..469097e484773 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -823,7 +823,7 @@ pub trait Iterator { /// /// Given an element the closure must return `true` or `false`. The returned /// iterator will yield only the elements for which the closure returns - /// true. + /// `true`. /// /// # Examples /// From 45e943fc0448ed431beb4f7573dd19774ab48bcb Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 26 Jul 2024 18:44:16 +0200 Subject: [PATCH 27/27] use attribute name that is incompatible in error message --- compiler/rustc_passes/messages.ftl | 2 +- compiler/rustc_passes/src/check_attr.rs | 1 + compiler/rustc_passes/src/errors.rs | 1 + tests/ui/asm/naked-functions-inline.stderr | 6 +++--- tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr | 4 ++-- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 4638ab62379e6..bfe0d54e64521 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -484,7 +484,7 @@ passes_naked_functions_asm_options = passes_naked_functions_incompatible_attribute = attribute incompatible with `#[naked]` - .label = this attribute is incompatible with `#[naked]` + .label = the `{$attr}` attribute is incompatible with `#[naked]` .naked_attribute = function marked with `#[naked]` here passes_naked_functions_must_use_noreturn = diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index f5cb674646684..0ac74e07b7b1d 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -461,6 +461,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { self.dcx().emit_err(errors::NakedFunctionIncompatibleAttribute { span: other_attr.span, naked_span: attr.span, + attr: other_attr.name_or_empty(), }); return false; diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 7b41ad3eb37e5..b195ba973ce29 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1190,6 +1190,7 @@ pub struct NakedFunctionIncompatibleAttribute { pub span: Span, #[label(passes_naked_attribute)] pub naked_span: Span, + pub attr: Symbol, } #[derive(Diagnostic)] diff --git a/tests/ui/asm/naked-functions-inline.stderr b/tests/ui/asm/naked-functions-inline.stderr index 9754551b90a88..f6eb31468ac37 100644 --- a/tests/ui/asm/naked-functions-inline.stderr +++ b/tests/ui/asm/naked-functions-inline.stderr @@ -4,7 +4,7 @@ error[E0736]: attribute incompatible with `#[naked]` LL | #[naked] | -------- function marked with `#[naked]` here LL | #[inline] - | ^^^^^^^^^ this attribute is incompatible with `#[naked]` + | ^^^^^^^^^ the `inline` attribute is incompatible with `#[naked]` error[E0736]: attribute incompatible with `#[naked]` --> $DIR/naked-functions-inline.rs:20:1 @@ -12,7 +12,7 @@ error[E0736]: attribute incompatible with `#[naked]` LL | #[naked] | -------- function marked with `#[naked]` here LL | #[inline(always)] - | ^^^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` + | ^^^^^^^^^^^^^^^^^ the `inline` attribute is incompatible with `#[naked]` error[E0736]: attribute incompatible with `#[naked]` --> $DIR/naked-functions-inline.rs:27:1 @@ -20,7 +20,7 @@ error[E0736]: attribute incompatible with `#[naked]` LL | #[naked] | -------- function marked with `#[naked]` here LL | #[inline(never)] - | ^^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` + | ^^^^^^^^^^^^^^^^ the `inline` attribute is incompatible with `#[naked]` error: aborting due to 3 previous errors diff --git a/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr b/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr index 1f2eb06aa577d..0625ed1183ba5 100644 --- a/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr +++ b/tests/ui/rfcs/rfc-2091-track-caller/error-with-naked.stderr @@ -2,7 +2,7 @@ error[E0736]: attribute incompatible with `#[naked]` --> $DIR/error-with-naked.rs:6:1 | LL | #[track_caller] - | ^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` + | ^^^^^^^^^^^^^^^ the `track_caller` attribute is incompatible with `#[naked]` LL | LL | #[naked] | -------- function marked with `#[naked]` here @@ -11,7 +11,7 @@ error[E0736]: attribute incompatible with `#[naked]` --> $DIR/error-with-naked.rs:18:5 | LL | #[track_caller] - | ^^^^^^^^^^^^^^^ this attribute is incompatible with `#[naked]` + | ^^^^^^^^^^^^^^^ the `track_caller` attribute is incompatible with `#[naked]` LL | LL | #[naked] | -------- function marked with `#[naked]` here