Skip to content

Commit

Permalink
Rollup merge of #91519 - petrochenkov:cratexp2, r=Aaron1011
Browse files Browse the repository at this point in the history
ast: Avoid aborts on fatal errors thrown from mutable AST visitor

Set the node to some dummy value and rethrow the error instead.

When using the old aborting `visit_clobber` in `InvocationCollector::visit_crate` the next tests abort due to fatal errors:
```
ui\modules\path-invalid-form.rs
ui\modules\path-macro.rs
ui\modules\path-no-file-name.rs
ui\parser\issues\issue-5806.rs
ui\parser\mod_file_with_path_attr.rs
```

Follow up to #91313.
  • Loading branch information
matthiaskrgr committed Dec 30, 2021
2 parents 65d8785 + bdb851f commit b9f7197
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 b9f7197

Please sign in to comment.