Do not abort compilation if expansion produces errors

Fix a number of uncovered deficiencies in diagnostics

Author: petrochenkov

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 {

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;

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() {

src/librustc_resolve/macros.rs

@@ -182,7 +182,14 @@ impl<'a> base::Resolver for Resolver<'a> {
         };
 
         let parent_scope = self.invoc_parent_scope(invoc_id, derives_in_scope);
-        let (def, ext) = self.resolve_macro_to_def(path, kind, &parent_scope, true, force)?;
+        let (def, ext) = match self.resolve_macro_to_def(path, kind, &parent_scope, true, force) {
+            Ok((def, ext)) => (def, ext),
+            Err(Determinacy::Determined) if kind == MacroKind::Attr => {
+                // Replace unresolved attributes with used inert attributes for better recovery.
+                return Ok(Some(self.get_macro(Def::NonMacroAttr(NonMacroAttrKind::Tool))));
+            }
+            Err(determinacy) => return Err(determinacy),
+        };
 
         if let Def::Macro(def_id, _) = def {
             if after_derive {
@@ -337,7 +344,6 @@ impl<'a> Resolver<'a> {
                 }
                 PathResult::Indeterminate if !force => return Err(Determinacy::Undetermined),
                 PathResult::NonModule(..) | PathResult::Indeterminate | PathResult::Failed(..) => {
-                    self.found_unresolved_macro = true;
                     Err(Determinacy::Determined)
                 }
                 PathResult::Module(..) => unreachable!(),
@@ -353,10 +359,8 @@ impl<'a> Resolver<'a> {
             let binding = self.early_resolve_ident_in_lexical_scope(
                 path[0].ident, ScopeSet::Macro(kind), parent_scope, false, force, path_span
             );
-            match binding {
-                Ok(..) => {}
-                Err(Determinacy::Determined) => self.found_unresolved_macro = true,
-                Err(Determinacy::Undetermined) => return Err(Determinacy::Undetermined),
+            if let Err(Determinacy::Undetermined) = binding {
+                return Err(Determinacy::Undetermined);
             }
 
             if trace {
@@ -858,14 +862,23 @@ impl<'a> Resolver<'a> {
     pub fn finalize_current_module_macro_resolutions(&mut self) {
         let module = self.current_module;
 
-        let check_consistency = |this: &mut Self, path: &[Segment], span,
-                                 kind: MacroKind, initial_def, def| {
+        let check_consistency = |this: &mut Self, path: &[Segment], span, kind: MacroKind,
+                                 initial_def: Option<Def>, def: Def| {
             if let Some(initial_def) = initial_def {
                 if def != initial_def && def != Def::Err && this.ambiguity_errors.is_empty() {
                     // Make sure compilation does not succeed if preferred macro resolution
                     // has changed after the macro had been expanded. In theory all such
                     // situations should be reported as ambiguity errors, so this is a bug.
-                    span_bug!(span, "inconsistent resolution for a macro");
+                    if initial_def == Def::NonMacroAttr(NonMacroAttrKind::Custom) {
+                        // Yeah, legacy custom attributes are implemented using forced resolution
+                        // (which is a best effort error recovery tool, basically), so we can't
+                        // promise their resolution won't change later.
+                        let msg = format!("inconsistent resolution for a macro: first {}, then {}",
+                                          initial_def.kind_name(), def.kind_name());
+                        this.session.span_err(span, &msg);
+                    } else {
+                        span_bug!(span, "inconsistent resolution for a macro");
+                    }
                 }
             } else {
                 // It's possible that the macro was unresolved (indeterminate) and silently

src/libsyntax/ext/base.rs

@@ -2,7 +2,7 @@ pub use self::SyntaxExtension::*;
 
 use ast::{self, Attribute, Name, PatKind, MetaItem};
 use attr::HasAttrs;
-use source_map::{self, SourceMap, Spanned, respan};
+use source_map::{SourceMap, Spanned, respan};
 use syntax_pos::{Span, MultiSpan, DUMMY_SP};
 use edition::Edition;
 use errors::{DiagnosticBuilder, DiagnosticId};
@@ -481,7 +481,7 @@ impl DummyResult {
     pub fn raw_expr(sp: Span) -> P<ast::Expr> {
         P(ast::Expr {
             id: ast::DUMMY_NODE_ID,
-            node: ast::ExprKind::Lit(source_map::respan(sp, ast::LitKind::Bool(false))),
+            node: ast::ExprKind::Err,
             span: sp,
             attrs: ThinVec::new(),
         })
@@ -496,10 +496,11 @@ impl DummyResult {
         }
     }
 
+    /// A plain dummy type.
     pub fn raw_ty(sp: Span) -> P<ast::Ty> {
         P(ast::Ty {
             id: ast::DUMMY_NODE_ID,
-            node: ast::TyKind::Infer,
+            node: ast::TyKind::Err,
             span: sp
         })
     }
@@ -796,7 +797,6 @@ pub struct ExtCtxt<'a> {
     pub ecfg: expand::ExpansionConfig<'a>,
     pub root_path: PathBuf,
     pub resolver: &'a mut dyn Resolver,
-    pub resolve_err_count: usize,
     pub current_expansion: ExpansionData,
     pub expansions: FxHashMap<Span, Vec<String>>,
 }
@@ -811,7 +811,6 @@ impl<'a> ExtCtxt<'a> {
             ecfg,
             root_path: PathBuf::new(),
             resolver,
-            resolve_err_count: 0,
             current_expansion: ExpansionData {
                 mark: Mark::root(),
                 depth: 0,

src/libsyntax/ext/expand.rs

@@ -344,8 +344,10 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
             // FIXME(jseyfried): Refactor out the following logic
             let (expanded_fragment, new_invocations) = if let Some(ext) = ext {
                 if let Some(ext) = ext {
-                    let dummy = invoc.fragment_kind.dummy(invoc.span()).unwrap();
-                    let fragment = self.expand_invoc(invoc, &*ext).unwrap_or(dummy);
+                    let (invoc_fragment_kind, invoc_span) = (invoc.fragment_kind, invoc.span());
+                    let fragment = self.expand_invoc(invoc, &*ext).unwrap_or_else(|| {
+                        invoc_fragment_kind.dummy(invoc_span).unwrap()
+                    });
                     self.collect_invocations(fragment, &[])
                 } else if let InvocationKind::Attr { attr: None, traits, item, .. } = invoc.kind {
                     if !item.derive_allowed() {
@@ -431,9 +433,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
 
     fn resolve_imports(&mut self) {
         if self.monotonic {
-            let err_count = self.cx.parse_sess.span_diagnostic.err_count();
             self.cx.resolver.resolve_imports();
-            self.cx.resolve_err_count += self.cx.parse_sess.span_diagnostic.err_count() - err_count;
         }
     }
 
@@ -457,11 +457,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
         };
 
         if self.monotonic {
-            let err_count = self.cx.parse_sess.span_diagnostic.err_count();
-            let mark = self.cx.current_expansion.mark;
-            self.cx.resolver.visit_ast_fragment_with_placeholders(mark, &fragment_with_placeholders,
-                                                                  derives);
-            self.cx.resolve_err_count += self.cx.parse_sess.span_diagnostic.err_count() - err_count;
+            self.cx.resolver.visit_ast_fragment_with_placeholders(
+                self.cx.current_expansion.mark, &fragment_with_placeholders, derives
+            );
         }
 
         (fragment_with_placeholders, invocations)

src/libsyntax/feature_gate.rs

@@ -1272,16 +1272,15 @@ impl<'a> Context<'a> {
                 return;
             }
         }
-        if name.starts_with("rustc_") {
-            gate_feature!(self, rustc_attrs, attr.span,
-                          "unless otherwise specified, attributes \
-                           with the prefix `rustc_` \
-                           are reserved for internal compiler diagnostics");
-        } else if !attr::is_known(attr) {
-            // Only run the custom attribute lint during regular feature gate
-            // checking. Macro gating runs before the plugin attributes are
-            // registered, so we skip this in that case.
-            if !is_macro {
+        if !attr::is_known(attr) {
+            if name.starts_with("rustc_") {
+                let msg = "unless otherwise specified, attributes with the prefix `rustc_` \
+                           are reserved for internal compiler diagnostics";
+                gate_feature!(self, rustc_attrs, attr.span, msg);
+            } else if !is_macro {
+                // Only run the custom attribute lint during regular feature gate
+                // checking. Macro gating runs before the plugin attributes are
+                // registered, so we skip this in that case.
                 let msg = format!("The attribute `{}` is currently unknown to the compiler and \
                                    may have meaning added to it in the future", attr.path);
                 gate_feature!(self, custom_attribute, attr.span, &msg);

src/libsyntax_ext/deriving/default.rs

@@ -3,7 +3,7 @@ use deriving::generic::*;
 use deriving::generic::ty::*;
 
 use syntax::ast::{Expr, MetaItem};
-use syntax::ext::base::{Annotatable, ExtCtxt};
+use syntax::ext::base::{Annotatable, DummyResult, ExtCtxt};
 use syntax::ext::build::AstBuilder;
 use syntax::ptr::P;
 use syntax::symbol::Symbol;
@@ -69,7 +69,7 @@ fn default_substructure(cx: &mut ExtCtxt, trait_span: Span, substr: &Substructur
             span_err!(cx, trait_span, E0665,
                       "`Default` cannot be derived for enums, only structs");
             // let compilation continue
-            cx.expr_usize(trait_span, 0)
+            DummyResult::raw_expr(trait_span)
         }
         _ => cx.span_bug(trait_span, "Non-static method in `derive(Default)`"),
     };

src/libsyntax_ext/format.rs

@@ -666,7 +666,7 @@ impl<'a, 'b> Context<'a, 'b> {
                     "X" => "UpperHex",
                     _ => {
                         ecx.span_err(sp, &format!("unknown format trait `{}`", *tyname));
-                        "Dummy"
+                        return DummyResult::raw_expr(sp);
                     }
                 }
             }
@@ -713,7 +713,7 @@ pub fn expand_format_args_nl<'cx>(
                                        sp,
                                        feature_gate::GateIssue::Language,
                                        feature_gate::EXPLAIN_FORMAT_ARGS_NL);
-        return base::DummyResult::expr(sp);
+        return DummyResult::expr(sp);
     }
     sp = sp.apply_mark(ecx.current_expansion.mark);
     match parse_args(ecx, sp, tts) {

src/test/ui/conditional-compilation/cfg-attr-invalid-predicate.rs

@@ -1,2 +1,4 @@
 #[cfg(foo(bar))] //~ ERROR invalid predicate `foo`
+fn check() {}
+
 fn main() {}