From 8e068b989bdbbf6eeeeee8ca2ba7db50c3e49dfc Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 9 Nov 2024 18:21:03 +0000 Subject: [PATCH 1/7] Recurse into APITs in impl_trait_overcaptures --- .../rustc_lint/src/impl_trait_overcaptures.rs | 6 ++- .../precise-capturing/overcaptures-2024.fixed | 5 ++ .../precise-capturing/overcaptures-2024.rs | 5 ++ .../overcaptures-2024.stderr | 51 +++++++++++++------ 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 036e0381a06..beab4d4e6a9 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -262,7 +262,11 @@ where // If it's owned by this function && let opaque = self.tcx.hir_node_by_def_id(opaque_def_id).expect_opaque_ty() - && let hir::OpaqueTyOrigin::FnReturn { parent, .. } = opaque.origin + // We want to recurse into RPITs and async fns, even though the latter + // doesn't overcapture on its own, it may mention additional RPITs + // in its bounds. + && let hir::OpaqueTyOrigin::FnReturn { parent, .. } + | hir::OpaqueTyOrigin::AsyncFn { parent, .. } = opaque.origin && parent == self.parent_def_id { let opaque_span = self.tcx.def_span(opaque_def_id); diff --git a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed index 6a9d72d028c..1eb88c71d54 100644 --- a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed @@ -1,4 +1,5 @@ //@ run-rustfix +//@ edition: 2018 #![allow(unused)] #![deny(impl_trait_overcaptures)] @@ -37,4 +38,8 @@ fn apit2(_: &T, _: U) -> impl Sized + use {} //~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 //~| WARN this changes meaning in Rust 2024 +async fn async_fn<'a>(x: &'a ()) -> impl Sized + use<> {} +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 +//~| WARN this changes meaning in Rust 2024 + fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs index 3a4f5ebb7fb..6f1ef6a472f 100644 --- a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs @@ -1,4 +1,5 @@ //@ run-rustfix +//@ edition: 2018 #![allow(unused)] #![deny(impl_trait_overcaptures)] @@ -37,4 +38,8 @@ fn apit2(_: &impl Sized, _: U) -> impl Sized {} //~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 //~| WARN this changes meaning in Rust 2024 +async fn async_fn<'a>(x: &'a ()) -> impl Sized {} +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 +//~| WARN this changes meaning in Rust 2024 + fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr index c101b980c71..63c87cd46c8 100644 --- a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr @@ -1,5 +1,5 @@ error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 - --> $DIR/overcaptures-2024.rs:6:29 + --> $DIR/overcaptures-2024.rs:7:29 | LL | fn named<'a>(x: &'a i32) -> impl Sized { *x } | ^^^^^^^^^^ @@ -7,13 +7,13 @@ LL | fn named<'a>(x: &'a i32) -> impl Sized { *x } = warning: this changes meaning in Rust 2024 = note: for more information, see note: specifically, this lifetime is in scope but not mentioned in the type's bounds - --> $DIR/overcaptures-2024.rs:6:10 + --> $DIR/overcaptures-2024.rs:7:10 | LL | fn named<'a>(x: &'a i32) -> impl Sized { *x } | ^^ = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 note: the lint level is defined here - --> $DIR/overcaptures-2024.rs:4:9 + --> $DIR/overcaptures-2024.rs:5:9 | LL | #![deny(impl_trait_overcaptures)] | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -23,7 +23,7 @@ LL | fn named<'a>(x: &'a i32) -> impl Sized + use<> { *x } | +++++++ error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 - --> $DIR/overcaptures-2024.rs:10:25 + --> $DIR/overcaptures-2024.rs:11:25 | LL | fn implicit(x: &i32) -> impl Sized { *x } | ^^^^^^^^^^ @@ -31,7 +31,7 @@ LL | fn implicit(x: &i32) -> impl Sized { *x } = warning: this changes meaning in Rust 2024 = note: for more information, see note: specifically, this lifetime is in scope but not mentioned in the type's bounds - --> $DIR/overcaptures-2024.rs:10:16 + --> $DIR/overcaptures-2024.rs:11:16 | LL | fn implicit(x: &i32) -> impl Sized { *x } | ^ @@ -42,7 +42,7 @@ LL | fn implicit(x: &i32) -> impl Sized + use<> { *x } | +++++++ error: `impl Sized + '_` will capture more lifetimes than possibly intended in edition 2024 - --> $DIR/overcaptures-2024.rs:16:33 + --> $DIR/overcaptures-2024.rs:17:33 | LL | fn hello(&self, x: &i32) -> impl Sized + '_ { self } | ^^^^^^^^^^^^^^^ @@ -50,7 +50,7 @@ LL | fn hello(&self, x: &i32) -> impl Sized + '_ { self } = warning: this changes meaning in Rust 2024 = note: for more information, see note: specifically, this lifetime is in scope but not mentioned in the type's bounds - --> $DIR/overcaptures-2024.rs:16:24 + --> $DIR/overcaptures-2024.rs:17:24 | LL | fn hello(&self, x: &i32) -> impl Sized + '_ { self } | ^ @@ -61,7 +61,7 @@ LL | fn hello(&self, x: &i32) -> impl Sized + '_ + use<'_> { self } | +++++++++ error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 - --> $DIR/overcaptures-2024.rs:28:47 + --> $DIR/overcaptures-2024.rs:29:47 | LL | fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized> {} | ^^^^^^^^^^ @@ -69,7 +69,7 @@ LL | fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized> {} = warning: this changes meaning in Rust 2024 = note: for more information, see note: specifically, this lifetime is in scope but not mentioned in the type's bounds - --> $DIR/overcaptures-2024.rs:28:23 + --> $DIR/overcaptures-2024.rs:29:23 | LL | fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized> {} | ^^ @@ -80,7 +80,7 @@ LL | fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized + use<>> {} | +++++++ error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 - --> $DIR/overcaptures-2024.rs:32:28 + --> $DIR/overcaptures-2024.rs:33:28 | LL | fn apit(_: &impl Sized) -> impl Sized {} | ^^^^^^^^^^ @@ -88,13 +88,13 @@ LL | fn apit(_: &impl Sized) -> impl Sized {} = warning: this changes meaning in Rust 2024 = note: for more information, see note: specifically, this lifetime is in scope but not mentioned in the type's bounds - --> $DIR/overcaptures-2024.rs:32:12 + --> $DIR/overcaptures-2024.rs:33:12 | LL | fn apit(_: &impl Sized) -> impl Sized {} | ^ = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 note: you could use a `use<...>` bound to explicitly specify captures, but argument-position `impl Trait`s are not nameable - --> $DIR/overcaptures-2024.rs:32:13 + --> $DIR/overcaptures-2024.rs:33:13 | LL | fn apit(_: &impl Sized) -> impl Sized {} | ^^^^^^^^^^ @@ -104,7 +104,7 @@ LL | fn apit(_: &T) -> impl Sized + use {} | ++++++++++ ~ ++++++++ error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 - --> $DIR/overcaptures-2024.rs:36:38 + --> $DIR/overcaptures-2024.rs:37:38 | LL | fn apit2(_: &impl Sized, _: U) -> impl Sized {} | ^^^^^^^^^^ @@ -112,13 +112,13 @@ LL | fn apit2(_: &impl Sized, _: U) -> impl Sized {} = warning: this changes meaning in Rust 2024 = note: for more information, see note: specifically, this lifetime is in scope but not mentioned in the type's bounds - --> $DIR/overcaptures-2024.rs:36:16 + --> $DIR/overcaptures-2024.rs:37:16 | LL | fn apit2(_: &impl Sized, _: U) -> impl Sized {} | ^ = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 note: you could use a `use<...>` bound to explicitly specify captures, but argument-position `impl Trait`s are not nameable - --> $DIR/overcaptures-2024.rs:36:17 + --> $DIR/overcaptures-2024.rs:37:17 | LL | fn apit2(_: &impl Sized, _: U) -> impl Sized {} | ^^^^^^^^^^ @@ -127,5 +127,24 @@ help: use the precise capturing `use<...>` syntax to make the captures explicit LL | fn apit2(_: &T, _: U) -> impl Sized + use {} | ++++++++++ ~ +++++++++++ -error: aborting due to 6 previous errors +error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + --> $DIR/overcaptures-2024.rs:41:37 + | +LL | async fn async_fn<'a>(x: &'a ()) -> impl Sized {} + | ^^^^^^^^^^ + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see +note: specifically, this lifetime is in scope but not mentioned in the type's bounds + --> $DIR/overcaptures-2024.rs:41:19 + | +LL | async fn async_fn<'a>(x: &'a ()) -> impl Sized {} + | ^^ + = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 +help: use the precise capturing `use<...>` syntax to make the captures explicit + | +LL | async fn async_fn<'a>(x: &'a ()) -> impl Sized + use<> {} + | +++++++ + +error: aborting due to 7 previous errors From 7aee2b332fb1a2b6c72893f425d650bd8c705bb6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 14 Nov 2024 13:32:22 +1100 Subject: [PATCH 2/7] Make `configure_annotatable`/`flat_map_annotatable` infallible. They each have a single callsite, and the result is always unwrapped, so the `Option` return type is misleading. Also, the comment at the `configure_annotatable` call site is wrong, talking about a result vector, so this commit also removes that. --- compiler/rustc_builtin_macros/src/cfg_eval.rs | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/cfg_eval.rs b/compiler/rustc_builtin_macros/src/cfg_eval.rs index b686a8cf935..2cb970aaa4d 100644 --- a/compiler/rustc_builtin_macros/src/cfg_eval.rs +++ b/compiler/rustc_builtin_macros/src/cfg_eval.rs @@ -39,46 +39,44 @@ pub(crate) fn cfg_eval( let features = Some(features); CfgEval(StripUnconfigured { sess, features, config_tokens: true, lint_node_id }) .configure_annotatable(annotatable) - // Since the item itself has already been configured by the `InvocationCollector`, - // we know that fold result vector will contain exactly one element. - .unwrap() } struct CfgEval<'a>(StripUnconfigured<'a>); -fn flat_map_annotatable( - vis: &mut impl MutVisitor, - annotatable: Annotatable, -) -> Option { +fn flat_map_annotatable(vis: &mut impl MutVisitor, annotatable: Annotatable) -> Annotatable { match annotatable { - Annotatable::Item(item) => vis.flat_map_item(item).pop().map(Annotatable::Item), + Annotatable::Item(item) => Annotatable::Item(vis.flat_map_item(item).pop().unwrap()), Annotatable::AssocItem(item, ctxt) => { - Some(Annotatable::AssocItem(vis.flat_map_assoc_item(item, ctxt).pop()?, ctxt)) + Annotatable::AssocItem(vis.flat_map_assoc_item(item, ctxt).pop().unwrap(), ctxt) } Annotatable::ForeignItem(item) => { - vis.flat_map_foreign_item(item).pop().map(Annotatable::ForeignItem) + Annotatable::ForeignItem(vis.flat_map_foreign_item(item).pop().unwrap()) } Annotatable::Stmt(stmt) => { - vis.flat_map_stmt(stmt.into_inner()).pop().map(P).map(Annotatable::Stmt) + Annotatable::Stmt(P(vis.flat_map_stmt(stmt.into_inner()).pop().unwrap())) } Annotatable::Expr(mut expr) => { vis.visit_expr(&mut expr); - Some(Annotatable::Expr(expr)) + Annotatable::Expr(expr) } - Annotatable::Arm(arm) => vis.flat_map_arm(arm).pop().map(Annotatable::Arm), + Annotatable::Arm(arm) => Annotatable::Arm(vis.flat_map_arm(arm).pop().unwrap()), Annotatable::ExprField(field) => { - vis.flat_map_expr_field(field).pop().map(Annotatable::ExprField) + Annotatable::ExprField(vis.flat_map_expr_field(field).pop().unwrap()) + } + Annotatable::PatField(fp) => { + Annotatable::PatField(vis.flat_map_pat_field(fp).pop().unwrap()) } - Annotatable::PatField(fp) => vis.flat_map_pat_field(fp).pop().map(Annotatable::PatField), Annotatable::GenericParam(param) => { - vis.flat_map_generic_param(param).pop().map(Annotatable::GenericParam) + Annotatable::GenericParam(vis.flat_map_generic_param(param).pop().unwrap()) } - Annotatable::Param(param) => vis.flat_map_param(param).pop().map(Annotatable::Param), - Annotatable::FieldDef(sf) => vis.flat_map_field_def(sf).pop().map(Annotatable::FieldDef), - Annotatable::Variant(v) => vis.flat_map_variant(v).pop().map(Annotatable::Variant), + Annotatable::Param(param) => Annotatable::Param(vis.flat_map_param(param).pop().unwrap()), + Annotatable::FieldDef(sf) => { + Annotatable::FieldDef(vis.flat_map_field_def(sf).pop().unwrap()) + } + Annotatable::Variant(v) => Annotatable::Variant(vis.flat_map_variant(v).pop().unwrap()), Annotatable::Crate(mut krate) => { vis.visit_crate(&mut krate); - Some(Annotatable::Crate(krate)) + Annotatable::Crate(krate) } } } @@ -123,11 +121,11 @@ impl CfgEval<'_> { self.0.configure(node) } - fn configure_annotatable(&mut self, mut annotatable: Annotatable) -> Option { + fn configure_annotatable(mut self, mut annotatable: Annotatable) -> Annotatable { // Tokenizing and re-parsing the `Annotatable` can have a significant // performance impact, so try to avoid it if possible if !has_cfg_or_cfg_attr(&annotatable) { - return Some(annotatable); + return annotatable; } // The majority of parsed attribute targets will never need to have early cfg-expansion @@ -197,13 +195,13 @@ impl CfgEval<'_> { Ok(a) => annotatable = a, Err(err) => { err.emit(); - return Some(annotatable); + return annotatable; } } // Now that we have our re-parsed `AttrTokenStream`, recursively configuring // our attribute target will correctly configure the tokens as well. - flat_map_annotatable(self, annotatable) + flat_map_annotatable(&mut self, annotatable) } } From b2b643c9d9d2a9807b5efe96e5902f99937c6c1e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 14 Nov 2024 13:58:27 +1100 Subject: [PATCH 3/7] Inline and remove `flat_map_annotatable`. Important: we know from the `parse_annotatable_with` call above the call site that only some of the `Annotatable` variants are possible. The remaining cases can be replaced with `unreachable!`. --- compiler/rustc_builtin_macros/src/cfg_eval.rs | 56 ++++++------------- 1 file changed, 17 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/cfg_eval.rs b/compiler/rustc_builtin_macros/src/cfg_eval.rs index 2cb970aaa4d..f0f534fe2e7 100644 --- a/compiler/rustc_builtin_macros/src/cfg_eval.rs +++ b/compiler/rustc_builtin_macros/src/cfg_eval.rs @@ -43,44 +43,6 @@ pub(crate) fn cfg_eval( struct CfgEval<'a>(StripUnconfigured<'a>); -fn flat_map_annotatable(vis: &mut impl MutVisitor, annotatable: Annotatable) -> Annotatable { - match annotatable { - Annotatable::Item(item) => Annotatable::Item(vis.flat_map_item(item).pop().unwrap()), - Annotatable::AssocItem(item, ctxt) => { - Annotatable::AssocItem(vis.flat_map_assoc_item(item, ctxt).pop().unwrap(), ctxt) - } - Annotatable::ForeignItem(item) => { - Annotatable::ForeignItem(vis.flat_map_foreign_item(item).pop().unwrap()) - } - Annotatable::Stmt(stmt) => { - Annotatable::Stmt(P(vis.flat_map_stmt(stmt.into_inner()).pop().unwrap())) - } - Annotatable::Expr(mut expr) => { - vis.visit_expr(&mut expr); - Annotatable::Expr(expr) - } - Annotatable::Arm(arm) => Annotatable::Arm(vis.flat_map_arm(arm).pop().unwrap()), - Annotatable::ExprField(field) => { - Annotatable::ExprField(vis.flat_map_expr_field(field).pop().unwrap()) - } - Annotatable::PatField(fp) => { - Annotatable::PatField(vis.flat_map_pat_field(fp).pop().unwrap()) - } - Annotatable::GenericParam(param) => { - Annotatable::GenericParam(vis.flat_map_generic_param(param).pop().unwrap()) - } - Annotatable::Param(param) => Annotatable::Param(vis.flat_map_param(param).pop().unwrap()), - Annotatable::FieldDef(sf) => { - Annotatable::FieldDef(vis.flat_map_field_def(sf).pop().unwrap()) - } - Annotatable::Variant(v) => Annotatable::Variant(vis.flat_map_variant(v).pop().unwrap()), - Annotatable::Crate(mut krate) => { - vis.visit_crate(&mut krate); - Annotatable::Crate(krate) - } - } -} - fn has_cfg_or_cfg_attr(annotatable: &Annotatable) -> bool { struct CfgFinder; @@ -201,7 +163,23 @@ impl CfgEval<'_> { // Now that we have our re-parsed `AttrTokenStream`, recursively configuring // our attribute target will correctly configure the tokens as well. - flat_map_annotatable(&mut self, annotatable) + match annotatable { + Annotatable::Item(item) => Annotatable::Item(self.flat_map_item(item).pop().unwrap()), + Annotatable::AssocItem(item, ctxt) => { + Annotatable::AssocItem(self.flat_map_assoc_item(item, ctxt).pop().unwrap(), ctxt) + } + Annotatable::ForeignItem(item) => { + Annotatable::ForeignItem(self.flat_map_foreign_item(item).pop().unwrap()) + } + Annotatable::Stmt(stmt) => { + Annotatable::Stmt(P(self.flat_map_stmt(stmt.into_inner()).pop().unwrap())) + } + Annotatable::Expr(mut expr) => { + self.visit_expr(&mut expr); + Annotatable::Expr(expr) + } + _ => unreachable!(), + } } } From d55a5e5ddad362408fe93d17c8e137cbd35dbf19 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 14 Nov 2024 14:18:56 +1100 Subject: [PATCH 4/7] Merge matches in `configure_annotatable`. There are two matches: one in a closure, and one vanilla one. They can be combined and simplified by putting them in a `try` block. --- compiler/rustc_builtin_macros/src/cfg_eval.rs | 102 ++++++++---------- 1 file changed, 45 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/cfg_eval.rs b/compiler/rustc_builtin_macros/src/cfg_eval.rs index f0f534fe2e7..b8a477b2b34 100644 --- a/compiler/rustc_builtin_macros/src/cfg_eval.rs +++ b/compiler/rustc_builtin_macros/src/cfg_eval.rs @@ -83,7 +83,7 @@ impl CfgEval<'_> { self.0.configure(node) } - fn configure_annotatable(mut self, mut annotatable: Annotatable) -> Annotatable { + fn configure_annotatable(mut self, annotatable: Annotatable) -> Annotatable { // Tokenizing and re-parsing the `Annotatable` can have a significant // performance impact, so try to avoid it if possible if !has_cfg_or_cfg_attr(&annotatable) { @@ -100,39 +100,6 @@ impl CfgEval<'_> { // the location of `#[cfg]` and `#[cfg_attr]` in the token stream. The tokenization // process is lossless, so this process is invisible to proc-macros. - let parse_annotatable_with: for<'a> fn(&mut Parser<'a>) -> PResult<'a, _> = - match annotatable { - Annotatable::Item(_) => { - |parser| Ok(Annotatable::Item(parser.parse_item(ForceCollect::Yes)?.unwrap())) - } - Annotatable::AssocItem(_, AssocCtxt::Trait) => |parser| { - Ok(Annotatable::AssocItem( - parser.parse_trait_item(ForceCollect::Yes)?.unwrap().unwrap(), - AssocCtxt::Trait, - )) - }, - Annotatable::AssocItem(_, AssocCtxt::Impl) => |parser| { - Ok(Annotatable::AssocItem( - parser.parse_impl_item(ForceCollect::Yes)?.unwrap().unwrap(), - AssocCtxt::Impl, - )) - }, - Annotatable::ForeignItem(_) => |parser| { - Ok(Annotatable::ForeignItem( - parser.parse_foreign_item(ForceCollect::Yes)?.unwrap().unwrap(), - )) - }, - Annotatable::Stmt(_) => |parser| { - Ok(Annotatable::Stmt(P(parser - .parse_stmt_without_recovery(false, ForceCollect::Yes)? - .unwrap()))) - }, - Annotatable::Expr(_) => { - |parser| Ok(Annotatable::Expr(parser.parse_expr_force_collect()?)) - } - _ => unreachable!(), - }; - // 'Flatten' all nonterminals (i.e. `TokenKind::Interpolated`) // to `None`-delimited groups containing the corresponding tokens. This // is normally delayed until the proc-macro server actually needs to @@ -151,35 +118,56 @@ impl CfgEval<'_> { // Re-parse the tokens, setting the `capture_cfg` flag to save extra information // to the captured `AttrTokenStream` (specifically, we capture // `AttrTokenTree::AttrsTarget` for all occurrences of `#[cfg]` and `#[cfg_attr]`) + // + // After that we have our re-parsed `AttrTokenStream`, recursively configuring + // our attribute target will correctly configure the tokens as well. let mut parser = Parser::new(&self.0.sess.psess, orig_tokens, None); parser.capture_cfg = true; - match parse_annotatable_with(&mut parser) { - Ok(a) => annotatable = a, + let res: PResult<'_, Annotatable> = try { + match annotatable { + Annotatable::Item(_) => { + let item = parser.parse_item(ForceCollect::Yes)?.unwrap(); + Annotatable::Item(self.flat_map_item(item).pop().unwrap()) + } + Annotatable::AssocItem(_, AssocCtxt::Trait) => { + let item = parser.parse_trait_item(ForceCollect::Yes)?.unwrap().unwrap(); + Annotatable::AssocItem( + self.flat_map_assoc_item(item, AssocCtxt::Trait).pop().unwrap(), + AssocCtxt::Trait, + ) + } + Annotatable::AssocItem(_, AssocCtxt::Impl) => { + let item = parser.parse_impl_item(ForceCollect::Yes)?.unwrap().unwrap(); + Annotatable::AssocItem( + self.flat_map_assoc_item(item, AssocCtxt::Impl).pop().unwrap(), + AssocCtxt::Impl, + ) + } + Annotatable::ForeignItem(_) => { + let item = parser.parse_foreign_item(ForceCollect::Yes)?.unwrap().unwrap(); + Annotatable::ForeignItem(self.flat_map_foreign_item(item).pop().unwrap()) + } + Annotatable::Stmt(_) => { + let stmt = + parser.parse_stmt_without_recovery(false, ForceCollect::Yes)?.unwrap(); + Annotatable::Stmt(P(self.flat_map_stmt(stmt).pop().unwrap())) + } + Annotatable::Expr(_) => { + let mut expr = parser.parse_expr_force_collect()?; + self.visit_expr(&mut expr); + Annotatable::Expr(expr) + } + _ => unreachable!(), + } + }; + + match res { + Ok(ann) => ann, Err(err) => { err.emit(); - return annotatable; + annotatable } } - - // Now that we have our re-parsed `AttrTokenStream`, recursively configuring - // our attribute target will correctly configure the tokens as well. - match annotatable { - Annotatable::Item(item) => Annotatable::Item(self.flat_map_item(item).pop().unwrap()), - Annotatable::AssocItem(item, ctxt) => { - Annotatable::AssocItem(self.flat_map_assoc_item(item, ctxt).pop().unwrap(), ctxt) - } - Annotatable::ForeignItem(item) => { - Annotatable::ForeignItem(self.flat_map_foreign_item(item).pop().unwrap()) - } - Annotatable::Stmt(stmt) => { - Annotatable::Stmt(P(self.flat_map_stmt(stmt.into_inner()).pop().unwrap())) - } - Annotatable::Expr(mut expr) => { - self.visit_expr(&mut expr); - Annotatable::Expr(expr) - } - _ => unreachable!(), - } } } From 7a96ed3dd0feb006dab9e4960204086dcac3dfb7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 14 Nov 2024 14:57:52 +1100 Subject: [PATCH 5/7] Remove unreachable code in `has_cfg_or_cfg_attr`. --- compiler/rustc_builtin_macros/src/cfg_eval.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/cfg_eval.rs b/compiler/rustc_builtin_macros/src/cfg_eval.rs index b8a477b2b34..f419c1e776b 100644 --- a/compiler/rustc_builtin_macros/src/cfg_eval.rs +++ b/compiler/rustc_builtin_macros/src/cfg_eval.rs @@ -66,14 +66,7 @@ fn has_cfg_or_cfg_attr(annotatable: &Annotatable) -> bool { Annotatable::ForeignItem(item) => CfgFinder.visit_foreign_item(item), Annotatable::Stmt(stmt) => CfgFinder.visit_stmt(stmt), Annotatable::Expr(expr) => CfgFinder.visit_expr(expr), - Annotatable::Arm(arm) => CfgFinder.visit_arm(arm), - Annotatable::ExprField(field) => CfgFinder.visit_expr_field(field), - Annotatable::PatField(field) => CfgFinder.visit_pat_field(field), - Annotatable::GenericParam(param) => CfgFinder.visit_generic_param(param), - Annotatable::Param(param) => CfgFinder.visit_param(param), - Annotatable::FieldDef(field) => CfgFinder.visit_field_def(field), - Annotatable::Variant(variant) => CfgFinder.visit_variant(variant), - Annotatable::Crate(krate) => CfgFinder.visit_crate(krate), + _ => unreachable!(), }; res.is_break() } From 9fde49b338c2b87fc8e071e8baa0bb429d55b1a9 Mon Sep 17 00:00:00 2001 From: maxcabrajac Date: Thu, 14 Nov 2024 17:07:46 -0300 Subject: [PATCH 6/7] Change visit_precise_capturing_arg so it returns a Self::Result --- compiler/rustc_ast/src/visit.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index 2f8115441de..58497d70a24 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -200,8 +200,8 @@ pub trait Visitor<'ast>: Sized { fn visit_param_bound(&mut self, bounds: &'ast GenericBound, _ctxt: BoundKind) -> Self::Result { walk_param_bound(self, bounds) } - fn visit_precise_capturing_arg(&mut self, arg: &'ast PreciseCapturingArg) { - walk_precise_capturing_arg(self, arg); + fn visit_precise_capturing_arg(&mut self, arg: &'ast PreciseCapturingArg) -> Self::Result { + walk_precise_capturing_arg(self, arg) } fn visit_poly_trait_ref(&mut self, t: &'ast PolyTraitRef) -> Self::Result { walk_poly_trait_ref(self, t) @@ -730,14 +730,10 @@ pub fn walk_param_bound<'a, V: Visitor<'a>>(visitor: &mut V, bound: &'a GenericB pub fn walk_precise_capturing_arg<'a, V: Visitor<'a>>( visitor: &mut V, arg: &'a PreciseCapturingArg, -) { +) -> V::Result { match arg { - PreciseCapturingArg::Lifetime(lt) => { - visitor.visit_lifetime(lt, LifetimeCtxt::GenericArg); - } - PreciseCapturingArg::Arg(path, id) => { - visitor.visit_path(path, *id); - } + PreciseCapturingArg::Lifetime(lt) => visitor.visit_lifetime(lt, LifetimeCtxt::GenericArg), + PreciseCapturingArg::Arg(path, id) => visitor.visit_path(path, *id), } } From 194471cbd4a679ed18b22adfebd7ba4650417a69 Mon Sep 17 00:00:00 2001 From: Kajetan Puchalski Date: Thu, 14 Nov 2024 16:46:55 +0000 Subject: [PATCH 7/7] tests: Test pac-ret flag merging on clang with LTO Extend the test for pac-ret with clang and LTO by checking that different branch protection flags are preserved after the LTO step. There was an issue in older LLVM versions that was causing this to behave incorrectly. Tests the LLVM behaviour added in: https://github.com/llvm/llvm-project/commit/1782810b8440144a0141c24192acbaeb55a1545d --- .../rmake.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/run-make/pointer-auth-link-with-c-lto-clang/rmake.rs b/tests/run-make/pointer-auth-link-with-c-lto-clang/rmake.rs index cf6e3d86377..0a2186b0953 100644 --- a/tests/run-make/pointer-auth-link-with-c-lto-clang/rmake.rs +++ b/tests/run-make/pointer-auth-link-with-c-lto-clang/rmake.rs @@ -10,14 +10,16 @@ //@ ignore-cross-compile // Reason: the compiled binary is executed -use run_make_support::{clang, env_var, llvm_ar, run, rustc, static_lib_name}; +use run_make_support::{clang, env_var, llvm_ar, llvm_objdump, run, rustc, static_lib_name}; + +static PAUTH_A_KEY_PATTERN: &'static str = "paciasp"; +static PAUTH_B_KEY_PATTERN: &'static str = "pacibsp"; fn main() { clang() .arg("-v") .lto("thin") - .arg("-mbranch-protection=bti+pac-ret+leaf") - .arg("-O2") + .arg("-mbranch-protection=bti+pac-ret+b-key+leaf") .arg("-c") .out_exe("test.o") .input("test.c") @@ -32,5 +34,15 @@ fn main() { .input("test.rs") .output("test.bin") .run(); + + // Check that both a-key and b-key pac-ret survived LTO + llvm_objdump() + .disassemble() + .input("test.bin") + .run() + .assert_stdout_contains_regex(PAUTH_A_KEY_PATTERN) + .assert_stdout_contains_regex(PAUTH_B_KEY_PATTERN); + + // Check that the binary actually runs run("test.bin"); }