Auto merge of rust-lang#56999 - petrochenkov:macrecov2, r=estebank

AST/HIR: Introduce `ExprKind::Err` for better error recovery in the front-end This way we can avoid aborting compilation if expansion produces errors and generate `ExprKind::Err`s instead.
sanxiyn · Dec 27, 2018 · f8caa32 · f8caa32
2 parents fb86d60 + bc16ede
commit f8caa32
Show file tree

Hide file tree

Showing 127 changed files with 712 additions and 367 deletions.
diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs
@@ -392,7 +392,8 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
 
             hir::ExprKind::Closure(..) |
             hir::ExprKind::Lit(..) |
-            hir::ExprKind::Path(_) => {
+            hir::ExprKind::Path(_) |
+            hir::ExprKind::Err => {
                 self.straightline(expr, pred, None::<hir::Expr>.iter())
             }
         }

diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs
@@ -1099,6 +1099,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
         ExprKind::Yield(ref subexpression) => {
             visitor.visit_expr(subexpression);
         }
+        ExprKind::Err => {}
     }
 }
 

diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
@@ -2709,7 +2709,6 @@ impl<'a> LoweringContext<'a> {
             rules: self.lower_block_check_mode(&b.rules),
             span: b.span,
             targeted_by_break,
-            recovered: b.recovered,
         })
     }
 
@@ -3781,7 +3780,6 @@ impl<'a> LoweringContext<'a> {
                                 rules: hir::DefaultBlock,
                                 span,
                                 targeted_by_break: false,
-                                recovered: blk.recovered,
                             });
                             P(self.expr_block(blk, ThinVec::new()))
                         }
@@ -4117,6 +4115,8 @@ impl<'a> LoweringContext<'a> {
                 hir::ExprKind::Yield(P(expr))
             }
 
+            ExprKind::Err => hir::ExprKind::Err,
+
             // Desugar `ExprIfLet`
             // from: `if let <pat> = <sub_expr> <body> [<else_opt>]`
             ExprKind::IfLet(ref pats, ref sub_expr, ref body, ref else_opt) => {
@@ -4821,7 +4821,6 @@ impl<'a> LoweringContext<'a> {
             rules: hir::DefaultBlock,
             span,
             targeted_by_break: false,
-            recovered: false,
         }
     }
 

diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs
@@ -807,11 +807,6 @@ pub struct Block {
     /// break out of this block early.
     /// Used by `'label: {}` blocks and by `catch` statements.
     pub targeted_by_break: bool,
-    /// If true, don't emit return value type errors as the parser had
-    /// to recover from a parse error so this block will not have an
-    /// appropriate type. A parse error will have been emitted so the
-    /// compilation will never succeed if this is true.
-    pub recovered: bool,
 }
 
 #[derive(Clone, RustcEncodable, RustcDecodable)]
@@ -1362,6 +1357,7 @@ impl Expr {
             ExprKind::Struct(..) => ExprPrecedence::Struct,
             ExprKind::Repeat(..) => ExprPrecedence::Repeat,
             ExprKind::Yield(..) => ExprPrecedence::Yield,
+            ExprKind::Err => ExprPrecedence::Err,
         }
     }
 
@@ -1412,7 +1408,8 @@ impl Expr {
             ExprKind::AddrOf(..) |
             ExprKind::Binary(..) |
             ExprKind::Yield(..) |
-            ExprKind::Cast(..) => {
+            ExprKind::Cast(..) |
+            ExprKind::Err => {
                 false
             }
         }
@@ -1525,6 +1522,9 @@ pub enum ExprKind {
 
     /// A suspension point for generators. This is `yield <expr>` in Rust.
     Yield(P<Expr>),
+
+    /// Placeholder for an expression that wasn't syntactically well formed in some way.
+    Err,
 }
 
 /// Optionally `Self`-qualified value/type path or associated extension.

diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs
@@ -430,7 +430,9 @@ impl<'a> State<'a> {
                 self.s.word("_")?;
             }
             hir::TyKind::Err => {
-                self.s.word("?")?;
+                self.popen()?;
+                self.s.word("/*ERROR*/")?;
+                self.pclose()?;
             }
         }
         self.end()
@@ -1540,6 +1542,11 @@ impl<'a> State<'a> {
                 self.word_space("yield")?;
                 self.print_expr_maybe_paren(&expr, parser::PREC_JUMP)?;
             }
+            hir::ExprKind::Err => {
+                self.popen()?;
+                self.s.word("/*ERROR*/")?;
+                self.pclose()?;
+            }
         }
         self.ann.post(self, AnnNode::Expr(expr))?;
         self.end()

diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs
@@ -410,7 +410,6 @@ impl_stable_hash_for!(struct hir::Block {
     rules,
     span,
     targeted_by_break,
-    recovered,
 });
 
 impl_stable_hash_for!(struct hir::Pat {
@@ -592,7 +591,8 @@ impl_stable_hash_for!(enum hir::ExprKind {
     InlineAsm(asm, inputs, outputs),
     Struct(path, fields, base),
     Repeat(val, times),
-    Yield(val)
+    Yield(val),
+    Err
 });
 
 impl_stable_hash_for!(enum hir::LocalSource {

diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs
@@ -222,24 +222,22 @@ impl<'a> LintLevelsBuilder<'a> {
                 match item.node {
                     ast::MetaItemKind::Word => {}  // actual lint names handled later
                     ast::MetaItemKind::NameValue(ref name_value) => {
-                        let gate_reasons = !self.sess.features_untracked().lint_reasons;
                         if item.ident == "reason" {
                             // found reason, reslice meta list to exclude it
                             metas = &metas[0..metas.len()-1];
                             // FIXME (#55112): issue unused-attributes lint if we thereby
                             // don't have any lint names (`#[level(reason = "foo")]`)
                             if let ast::LitKind::Str(rationale, _) = name_value.node {
-                                if gate_reasons {
+                                if !self.sess.features_untracked().lint_reasons {
                                     feature_gate::emit_feature_err(
                                         &self.sess.parse_sess,
                                         "lint_reasons",
                                         item.span,
                                         feature_gate::GateIssue::Language,
                                         "lint reasons are experimental"
                                     );
-                                } else {
-                                    reason = Some(rationale);
                                 }
+                                reason = Some(rationale);
                             } else {
                                 let mut err = bad_attr(name_value.span);
                                 err.help("reason must be a string literal");

diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs
@@ -479,7 +479,8 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
             }
 
             hir::ExprKind::Continue(..) |
-            hir::ExprKind::Lit(..) => {}
+            hir::ExprKind::Lit(..) |
+            hir::ExprKind::Err => {}
 
             hir::ExprKind::Loop(ref blk, _, _) => {
                 self.walk_block(&blk);

diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs
@@ -515,6 +515,7 @@ fn visit_expr<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, expr: &'tcx Expr) {
       hir::ExprKind::Box(..) |
       hir::ExprKind::Yield(..) |
       hir::ExprKind::Type(..) |
+      hir::ExprKind::Err |
       hir::ExprKind::Path(hir::QPath::TypeRelative(..)) => {
           intravisit::walk_expr(ir, expr);
       }
@@ -1254,7 +1255,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
                 self.propagate_through_exprs(inputs, succ)
             }
 
-            hir::ExprKind::Lit(..) | hir::ExprKind::Path(hir::QPath::TypeRelative(..)) => {
+            hir::ExprKind::Lit(..) | hir::ExprKind::Err |
+            hir::ExprKind::Path(hir::QPath::TypeRelative(..)) => {
                 succ
             }
 
@@ -1521,7 +1523,7 @@ fn check_expr<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, expr: &'tcx Expr) {
         hir::ExprKind::Block(..) | hir::ExprKind::AddrOf(..) |
         hir::ExprKind::Struct(..) | hir::ExprKind::Repeat(..) |
         hir::ExprKind::Closure(..) | hir::ExprKind::Path(_) | hir::ExprKind::Yield(..) |
-        hir::ExprKind::Box(..) | hir::ExprKind::Type(..) => {
+        hir::ExprKind::Box(..) | hir::ExprKind::Type(..) | hir::ExprKind::Err => {
             intravisit::walk_expr(this, expr);
         }
     }

diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs
@@ -687,7 +687,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
             hir::ExprKind::Block(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Match(..) |
             hir::ExprKind::Lit(..) | hir::ExprKind::Break(..) |
             hir::ExprKind::Continue(..) | hir::ExprKind::Struct(..) | hir::ExprKind::Repeat(..) |
-            hir::ExprKind::InlineAsm(..) | hir::ExprKind::Box(..) => {
+            hir::ExprKind::InlineAsm(..) | hir::ExprKind::Box(..) | hir::ExprKind::Err => {
                 Ok(self.cat_rvalue_node(expr.hir_id, expr.span, expr_ty))
             }
         }

diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs
@@ -987,7 +987,6 @@ where
         };
 
         let mut ecx = ExtCtxt::new(&sess.parse_sess, cfg, &mut resolver);
-        let err_count = ecx.parse_sess.span_diagnostic.err_count();
 
         // Expand macros now!
         let krate = time(sess, "expand crate", || {
@@ -1013,9 +1012,6 @@ where
             let msg = "missing fragment specifier";
             sess.buffer_lint(lint, ast::CRATE_NODE_ID, span, msg);
         }
-        if ecx.parse_sess.span_diagnostic.err_count() - ecx.resolve_err_count > err_count {
-            ecx.parse_sess.span_diagnostic.abort_if_errors();
-        }
         if cfg!(windows) {
             env::set_var("PATH", &old_path);
         }
@@ -1119,12 +1115,6 @@ where
         })
     })?;
 
-    // Unresolved macros might be due to mistyped `#[macro_use]`,
-    // so abort after checking for unknown attributes. (#49074)
-    if resolver.found_unresolved_macro {
-        sess.diagnostic().abort_if_errors();
-    }
-
     // Lower ast -> hir.
     // First, we need to collect the dep_graph.
     let dep_graph = match future_dep_graph {

diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs
@@ -741,15 +741,13 @@ impl<'a> fold::Folder for ReplaceBodyWithLoop<'a> {
 
     fn fold_block(&mut self, b: P<ast::Block>) -> P<ast::Block> {
         fn stmt_to_block(rules: ast::BlockCheckMode,
-                         recovered: bool,
                          s: Option<ast::Stmt>,
                          sess: &Session) -> ast::Block {
             ast::Block {
                 stmts: s.into_iter().collect(),
                 rules,
                 id: sess.next_node_id(),
                 span: syntax_pos::DUMMY_SP,
-                recovered,
             }
         }
 
@@ -768,7 +766,7 @@ impl<'a> fold::Folder for ReplaceBodyWithLoop<'a> {
             }
         }
 
-        let empty_block = stmt_to_block(BlockCheckMode::Default, false, None, self.sess);
+        let empty_block = stmt_to_block(BlockCheckMode::Default, None, self.sess);
         let loop_expr = P(ast::Expr {
             node: ast::ExprKind::Loop(P(empty_block), None),
             id: self.sess.next_node_id(),
@@ -809,7 +807,7 @@ impl<'a> fold::Folder for ReplaceBodyWithLoop<'a> {
                         old_blocks.push(new_block);
                     }
 
-                    stmt_to_block(b.rules, b.recovered, Some(loop_stmt), self.sess)
+                    stmt_to_block(b.rules, Some(loop_stmt), self.sess)
                 } else {
                     //push `loop {}` onto the end of our fresh block and yield that
                     new_block.stmts.push(loop_stmt);

diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs
@@ -780,6 +780,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
         hir::ExprKind::Tup(ref fields) => ExprKind::Tuple { fields: fields.to_ref() },
 
         hir::ExprKind::Yield(ref v) => ExprKind::Yield { value: v.to_ref() },
+        hir::ExprKind::Err => unreachable!(),
     };
 
     Expr {

diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs
@@ -278,25 +278,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         visit::walk_ty(self, ty)
     }
 
-    fn visit_use_tree(&mut self, use_tree: &'a UseTree, id: NodeId, _nested: bool) {
-        // Check if the path in this `use` is not generic, such as `use foo::bar<T>;` While this
-        // can't happen normally thanks to the parser, a generic might sneak in if the `use` is
-        // built using a macro.
-        //
-        // macro_use foo {
-        //     ($p:path) => { use $p; }
-        // }
-        // foo!(bar::baz<T>);
-        use_tree.prefix.segments.iter().find(|segment| {
-            segment.args.is_some()
-        }).map(|segment| {
-            self.err_handler().span_err(segment.args.as_ref().unwrap().span(),
-                                        "generic arguments in import path");
-        });
-
-        visit::walk_use_tree(self, use_tree, id);
-    }
-
     fn visit_label(&mut self, label: &'a Label) {
         self.check_label(label.ident);
         visit::walk_label(self, label);
@@ -433,17 +414,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         visit::walk_foreign_item(self, fi)
     }
 
-    fn visit_vis(&mut self, vis: &'a Visibility) {
-        if let VisibilityKind::Restricted { ref path, .. } = vis.node {
-            path.segments.iter().find(|segment| segment.args.is_some()).map(|segment| {
-                self.err_handler().span_err(segment.args.as_ref().unwrap().span(),
-                                            "generic arguments in visibility path");
-            });
-        }
-
-        visit::walk_vis(self, vis)
-    }
-
     fn visit_generics(&mut self, generics: &'a Generics) {
         let mut seen_non_lifetime_param = false;
         let mut seen_default = None;

diff --git a/src/librustc_passes/rvalue_promotion.rs b/src/librustc_passes/rvalue_promotion.rs
@@ -449,7 +449,8 @@ fn check_expr_kind<'a, 'tcx>(
             struct_result
         }
 
-        hir::ExprKind::Lit(_) => Promotable,
+        hir::ExprKind::Lit(_) |
+        hir::ExprKind::Err => Promotable,
 
         hir::ExprKind::AddrOf(_, ref expr) |
         hir::ExprKind::Repeat(ref expr, _) => {

diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
@@ -1576,7 +1576,6 @@ pub struct Resolver<'a> {
     macro_map: FxHashMap<DefId, Lrc<SyntaxExtension>>,
     macro_defs: FxHashMap<Mark, DefId>,
     local_macro_def_scopes: FxHashMap<NodeId, Module<'a>>,
-    pub found_unresolved_macro: bool,
 
     /// List of crate local macros that we need to warn about as being unused.
     /// Right now this only includes macro_rules! macros, and macros 2.0.
@@ -1911,7 +1910,6 @@ impl<'a> Resolver<'a> {
             name_already_seen: FxHashMap::default(),
             potentially_unused_imports: Vec::new(),
             struct_constructors: Default::default(),
-            found_unresolved_macro: false,
             unused_macros: FxHashSet::default(),
             current_type_ascription: Vec::new(),
             injected_crate: None,
@@ -2024,8 +2022,10 @@ impl<'a> Resolver<'a> {
                                       record_used_id: Option<NodeId>,
                                       path_span: Span)
                                       -> Option<LexicalScopeBinding<'a>> {
-        let record_used = record_used_id.is_some();
         assert!(ns == TypeNS  || ns == ValueNS);
+        if ident.name == keywords::Invalid.name() {
+            return Some(LexicalScopeBinding::Def(Def::Err));
+        }
         if ns == TypeNS {
             ident.span = if ident.name == keywords::SelfUpper.name() {
                 // FIXME(jseyfried) improve `Self` hygiene
@@ -2038,6 +2038,7 @@ impl<'a> Resolver<'a> {
         }
 
         // Walk backwards up the ribs in scope.
+        let record_used = record_used_id.is_some();
         let mut module = self.graph_root;
         for i in (0 .. self.ribs[ns].len()).rev() {
             if let Some(def) = self.ribs[ns][i].bindings.get(&ident).cloned() {