From bdb851f567ed6123172b849abbfc9163d268e291 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 17 Oct 2021 19:32:34 +0300 Subject: [PATCH] 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) {