Skip to content

Commit

Permalink
ast: Avoid aborts on fatal errors thrown from mutable AST visitor
Browse files Browse the repository at this point in the history
Set the node to some dummy value and rethwor the error instead.
  • Loading branch information
petrochenkov committed Dec 4, 2021
1 parent 887999d commit bdb851f
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 21 deletions.
128 changes: 116 additions & 12 deletions compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<A: Array> {
fn expect_one(self, err: &'static str) -> A::Item;
Expand Down Expand Up @@ -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, F>(t: &mut T, f: F)
where
F: FnOnce(T) -> T,
{
pub fn visit_clobber<T: DummyAstNode>(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);
}
}
Expand Down Expand Up @@ -1454,3 +1453,108 @@ pub fn noop_visit_vis<T: MutVisitor>(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<T> DummyAstNode for Option<T> {
fn dummy() -> Self {
Default::default()
}
}

impl<T: DummyAstNode + 'static> DummyAstNode for P<T> {
fn dummy() -> Self {
P(DummyAstNode::dummy())
}
}

impl<T> DummyAstNode for ThinVec<T> {
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(),
}
}
}
20 changes: 11 additions & 9 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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<ast::Expr>) {
Expand Down

0 comments on commit bdb851f

Please sign in to comment.