From bdb851f567ed6123172b849abbfc9163d268e291 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 17 Oct 2021 19:32:34 +0300 Subject: [PATCH 1/5] ast: Avoid aborts on fatal errors thrown from mutable AST visitor Set the node to some dummy value and rethwor the error instead. --- compiler/rustc_ast/src/mut_visit.rs | 128 +++++++++++++++++++++++++--- compiler/rustc_expand/src/expand.rs | 20 +++-- 2 files changed, 127 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 205625573a6d9..7fd9d5b20b1c4 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -14,13 +14,14 @@ use crate::tokenstream::*; use rustc_data_structures::map_in_place::MapInPlace; use rustc_data_structures::sync::Lrc; +use rustc_data_structures::thin_vec::ThinVec; use rustc_span::source_map::Spanned; use rustc_span::symbol::Ident; use rustc_span::Span; use smallvec::{smallvec, Array, SmallVec}; use std::ops::DerefMut; -use std::{panic, process, ptr}; +use std::{panic, ptr}; pub trait ExpectOne { fn expect_one(self, err: &'static str) -> A::Item; @@ -283,23 +284,21 @@ pub trait MutVisitor: Sized { /// Use a map-style function (`FnOnce(T) -> T`) to overwrite a `&mut T`. Useful /// when using a `flat_map_*` or `filter_map_*` method within a `visit_` -/// method. Abort the program if the closure panics. -/// -/// FIXME: Abort on panic means that any fatal error inside `visit_clobber` will abort the compiler. -/// Instead of aborting on catching a panic we need to reset the visited node to some valid but -/// possibly meaningless value and rethrow the panic. +/// method. // // No `noop_` prefix because there isn't a corresponding method in `MutVisitor`. -pub fn visit_clobber(t: &mut T, f: F) -where - F: FnOnce(T) -> T, -{ +pub fn visit_clobber(t: &mut T, f: impl FnOnce(T) -> T) { unsafe { // Safe because `t` is used in a read-only fashion by `read()` before // being overwritten by `write()`. let old_t = ptr::read(t); - let new_t = panic::catch_unwind(panic::AssertUnwindSafe(|| f(old_t))) - .unwrap_or_else(|_| process::abort()); + let new_t = + panic::catch_unwind(panic::AssertUnwindSafe(|| f(old_t))).unwrap_or_else(|err| { + // Set `t` to some valid but possible meaningless value, + // and pass the fatal error further. + ptr::write(t, T::dummy()); + panic::resume_unwind(err); + }); ptr::write(t, new_t); } } @@ -1454,3 +1453,108 @@ pub fn noop_visit_vis(visibility: &mut Visibility, vis: &mut T) { } vis.visit_span(&mut visibility.span); } + +/// Some value for the AST node that is valid but possibly meaningless. +pub trait DummyAstNode { + fn dummy() -> Self; +} + +impl DummyAstNode for Option { + fn dummy() -> Self { + Default::default() + } +} + +impl DummyAstNode for P { + fn dummy() -> Self { + P(DummyAstNode::dummy()) + } +} + +impl DummyAstNode for ThinVec { + fn dummy() -> Self { + Default::default() + } +} + +impl DummyAstNode for Item { + fn dummy() -> Self { + Item { + attrs: Default::default(), + id: DUMMY_NODE_ID, + span: Default::default(), + vis: Visibility { + kind: VisibilityKind::Public, + span: Default::default(), + tokens: Default::default(), + }, + ident: Ident::empty(), + kind: ItemKind::ExternCrate(None), + tokens: Default::default(), + } + } +} + +impl DummyAstNode for Expr { + fn dummy() -> Self { + Expr { + id: DUMMY_NODE_ID, + kind: ExprKind::Err, + span: Default::default(), + attrs: Default::default(), + tokens: Default::default(), + } + } +} + +impl DummyAstNode for Ty { + fn dummy() -> Self { + Ty { + id: DUMMY_NODE_ID, + kind: TyKind::Err, + span: Default::default(), + tokens: Default::default(), + } + } +} + +impl DummyAstNode for Pat { + fn dummy() -> Self { + Pat { + id: DUMMY_NODE_ID, + kind: PatKind::Wild, + span: Default::default(), + tokens: Default::default(), + } + } +} + +impl DummyAstNode for Stmt { + fn dummy() -> Self { + Stmt { id: DUMMY_NODE_ID, kind: StmtKind::Empty, span: Default::default() } + } +} + +impl DummyAstNode for Block { + fn dummy() -> Self { + Block { + stmts: Default::default(), + id: DUMMY_NODE_ID, + rules: BlockCheckMode::Default, + span: Default::default(), + tokens: Default::default(), + could_be_bare_literal: Default::default(), + } + } +} + +impl DummyAstNode for Crate { + fn dummy() -> Self { + Crate { + attrs: Default::default(), + items: Default::default(), + span: Default::default(), + is_placeholder: Default::default(), + } + } +} diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index ba675daac41db..f216a66148703 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1160,13 +1160,18 @@ macro_rules! assign_id { impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { fn visit_crate(&mut self, krate: &mut ast::Crate) { - let span = krate.span; - let empty_crate = - || ast::Crate { attrs: Vec::new(), items: Vec::new(), span, is_placeholder: None }; - let mut fold_crate = |krate: ast::Crate| { + visit_clobber(krate, |krate| { + let span = krate.span; let mut krate = match self.configure(krate) { Some(krate) => krate, - None => return empty_crate(), + None => { + return ast::Crate { + attrs: Vec::new(), + items: Vec::new(), + span, + is_placeholder: None, + }; + } }; if let Some(attr) = self.take_first_attr(&mut krate) { @@ -1177,10 +1182,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { noop_visit_crate(&mut krate, self); krate - }; - - // Cannot use `visit_clobber` here, see the FIXME on it. - *krate = fold_crate(mem::replace(krate, empty_crate())); + }) } fn visit_expr(&mut self, expr: &mut P) { From b7df49895ce9868f7de5ce00d804131f7aff9426 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 29 Dec 2021 11:35:50 -0800 Subject: [PATCH 2/5] Fix spacing of pretty printed const item without body --- compiler/rustc_ast_pretty/src/pprust/state.rs | 2 +- .../issue-68710-field-attr-proc-mac-lost.rs | 3 ++- .../pretty/nested-item-vis-defaultness.rs | 20 +++++++++---------- src/test/ui/macros/stringify.rs | 6 +++--- src/test/ui/proc-macro/quote-debug.stdout | 3 ++- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index f0c1d017326cd..6c4f38e9f63cd 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1116,9 +1116,9 @@ impl<'a> State<'a> { self.print_ident(ident); self.word_space(":"); self.print_type(ty); - self.space(); self.end(); // end the head-ibox if let Some(body) = body { + self.space(); self.word_space("="); self.print_expr(body); } diff --git a/src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs b/src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs index ed7879001d559..5dd04a569e743 100644 --- a/src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs +++ b/src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs @@ -7,7 +7,8 @@ struct C { } #[allow()] -const C: C = +const C: C + = C{ #[cfg(debug_assertions)] field: 0, diff --git a/src/test/pretty/nested-item-vis-defaultness.rs b/src/test/pretty/nested-item-vis-defaultness.rs index f46c0e3f1bc62..b094ba577db2d 100644 --- a/src/test/pretty/nested-item-vis-defaultness.rs +++ b/src/test/pretty/nested-item-vis-defaultness.rs @@ -6,42 +6,42 @@ fn main() {} #[cfg(FALSE)] extern "C" { - static X: u8 ; + static X: u8; type X; fn foo(); - pub static X: u8 ; + pub static X: u8; pub type X; pub fn foo(); } #[cfg(FALSE)] trait T { - const X: u8 ; + const X: u8; type X; fn foo(); - default const X: u8 ; + default const X: u8; default type X; default fn foo(); - pub const X: u8 ; + pub const X: u8; pub type X; pub fn foo(); - pub default const X: u8 ; + pub default const X: u8; pub default type X; pub default fn foo(); } #[cfg(FALSE)] impl T for S { - const X: u8 ; + const X: u8; type X; fn foo(); - default const X: u8 ; + default const X: u8; default type X; default fn foo(); - pub const X: u8 ; + pub const X: u8; pub type X; pub fn foo(); - pub default const X: u8 ; + pub default const X: u8; pub default type X; pub default fn foo(); } diff --git a/src/test/ui/macros/stringify.rs b/src/test/ui/macros/stringify.rs index 90bc7dc1da239..7d1c05a85bcf5 100644 --- a/src/test/ui/macros/stringify.rs +++ b/src/test/ui/macros/stringify.rs @@ -382,13 +382,13 @@ fn test_item() { stringify_item!( static S: (); ), - "static S: () ;", // FIXME + "static S: ();", ); assert_eq!( stringify_item!( static mut S: (); ), - "static mut S: () ;", + "static mut S: ();", ); // ItemKind::Const @@ -402,7 +402,7 @@ fn test_item() { stringify_item!( const S: (); ), - "const S: () ;", // FIXME + "const S: ();", ); // ItemKind::Fn diff --git a/src/test/ui/proc-macro/quote-debug.stdout b/src/test/ui/proc-macro/quote-debug.stdout index 4bdc04b9ac430..d806d7c9aadd5 100644 --- a/src/test/ui/proc-macro/quote-debug.stdout +++ b/src/test/ui/proc-macro/quote-debug.stdout @@ -43,7 +43,8 @@ fn main() { crate::TokenStream::from(crate::TokenTree::Punct(crate::Punct::new('\u{3b}', crate::Spacing::Alone)))].iter().cloned().collect::() } -const _: () = +const _: () + = { extern crate proc_macro; #[rustc_proc_macro_decls] From b62163515ad109dad05985b958c98a001dbcd89b Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 29 Dec 2021 11:53:03 -0800 Subject: [PATCH 3/5] Move equal sign back into head ibox --- compiler/rustc_ast_pretty/src/pprust/state.rs | 4 +++- src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs | 3 +-- src/test/ui/proc-macro/quote-debug.stdout | 3 +-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index 6c4f38e9f63cd..0d5ce07c6ca11 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1116,9 +1116,11 @@ impl<'a> State<'a> { self.print_ident(ident); self.word_space(":"); self.print_type(ty); + if body.is_some() { + self.space(); + } self.end(); // end the head-ibox if let Some(body) = body { - self.space(); self.word_space("="); self.print_expr(body); } diff --git a/src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs b/src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs index 5dd04a569e743..ed7879001d559 100644 --- a/src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs +++ b/src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs @@ -7,8 +7,7 @@ struct C { } #[allow()] -const C: C - = +const C: C = C{ #[cfg(debug_assertions)] field: 0, diff --git a/src/test/ui/proc-macro/quote-debug.stdout b/src/test/ui/proc-macro/quote-debug.stdout index d806d7c9aadd5..4bdc04b9ac430 100644 --- a/src/test/ui/proc-macro/quote-debug.stdout +++ b/src/test/ui/proc-macro/quote-debug.stdout @@ -43,8 +43,7 @@ fn main() { crate::TokenStream::from(crate::TokenTree::Punct(crate::Punct::new('\u{3b}', crate::Spacing::Alone)))].iter().cloned().collect::() } -const _: () - = +const _: () = { extern crate proc_macro; #[rustc_proc_macro_decls] From 12b59b40272da2c004dbb1815b9e39e99e88cd30 Mon Sep 17 00:00:00 2001 From: Wang Ruochen Date: Wed, 29 Dec 2021 16:04:44 -0800 Subject: [PATCH 4/5] Add UI test for #92292 Closes #92292 --- src/test/ui/traits/issue-92292.rs | 32 +++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 src/test/ui/traits/issue-92292.rs diff --git a/src/test/ui/traits/issue-92292.rs b/src/test/ui/traits/issue-92292.rs new file mode 100644 index 0000000000000..bb3700a2b5e94 --- /dev/null +++ b/src/test/ui/traits/issue-92292.rs @@ -0,0 +1,32 @@ +// check-pass + +use std::marker::PhantomData; + +pub struct MyGenericType { + _marker: PhantomData<*const T>, +} + +pub struct MyNonGenericType; + +impl From> for MyNonGenericType { + fn from(_: MyGenericType) -> Self { + todo!() + } +} + +pub trait MyTrait { + const MY_CONSTANT: i32; +} + +impl MyTrait for MyGenericType +where + Self: Into, +{ + const MY_CONSTANT: i32 = 1; +} + +impl MyGenericType { + const MY_OTHER_CONSTANT: i32 = as MyTrait>::MY_CONSTANT; +} + +fn main() {} From e86ecdf9fe2d60dad52c1e60657ed749c2196c5b Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Thu, 30 Dec 2021 05:04:44 +0200 Subject: [PATCH 5/5] Use `UnsafeCell::get_mut()` in `core::lazy::OnceCell::get_mut()` This removes one unnecessary `unsafe` block. --- library/core/src/lazy.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/core/src/lazy.rs b/library/core/src/lazy.rs index 2b8a5f3cbf345..788f0cce01ba8 100644 --- a/library/core/src/lazy.rs +++ b/library/core/src/lazy.rs @@ -102,8 +102,7 @@ impl OnceCell { /// Returns `None` if the cell is empty. #[unstable(feature = "once_cell", issue = "74465")] pub fn get_mut(&mut self) -> Option<&mut T> { - // SAFETY: Safe because we have unique access - unsafe { &mut *self.inner.get() }.as_mut() + self.inner.get_mut().as_mut() } /// Sets the contents of the cell to `value`.