resolve/expand: resolve_macro_invocation no longer returns determinate errors

It either returns the indeterminacy error, or valid (but perhaps dummy) `SyntaxExtension`.

With this change enum `Determinacy` is no longer used in libsyntax and can be moved to resolve.

The regressions in diagnosics are fixed in the next commits.

Author: petrochenkov

src/librustc_resolve/build_reduced_graph.rs

@@ -9,7 +9,7 @@ use crate::resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleIm
 use crate::{Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, Segment, ToNameBinding};
 use crate::{ModuleOrUniformRoot, PerNS, Resolver, ResolverArenas, ExternPreludeEntry};
 use crate::Namespace::{self, TypeNS, ValueNS, MacroNS};
-use crate::{resolve_error, resolve_struct_error, ResolutionError};
+use crate::{resolve_error, resolve_struct_error, ResolutionError, Determinacy};
 
 use rustc::bug;
 use rustc::hir::def::{self, *};
@@ -30,7 +30,6 @@ use syntax::attr;
 use syntax::ast::{self, Block, ForeignItem, ForeignItemKind, Item, ItemKind, NodeId};
 use syntax::ast::{MetaItemKind, StmtKind, TraitItem, TraitItemKind, Variant};
 use syntax::ext::base::SyntaxExtension;
-use syntax::ext::base::Determinacy::Undetermined;
 use syntax::ext::hygiene::Mark;
 use syntax::ext::tt::macro_rules;
 use syntax::feature_gate::is_builtin_attr;
@@ -231,9 +230,9 @@ impl<'a> Resolver<'a> {
                     source: source.ident,
                     target: ident,
                     source_bindings: PerNS {
-                        type_ns: Cell::new(Err(Undetermined)),
-                        value_ns: Cell::new(Err(Undetermined)),
-                        macro_ns: Cell::new(Err(Undetermined)),
+                        type_ns: Cell::new(Err(Determinacy::Undetermined)),
+                        value_ns: Cell::new(Err(Determinacy::Undetermined)),
+                        macro_ns: Cell::new(Err(Determinacy::Undetermined)),
                     },
                     target_bindings: PerNS {
                         type_ns: Cell::new(None),

src/librustc_resolve/lib.rs

@@ -15,6 +15,7 @@
 
 pub use rustc::hir::def::{Namespace, PerNS};
 
+use Determinacy::*;
 use GenericParameters::*;
 use RibKind::*;
 use smallvec::smallvec;
@@ -41,7 +42,6 @@ use syntax::source_map::SourceMap;
 use syntax::ext::hygiene::{Mark, Transparency, SyntaxContext};
 use syntax::ast::{self, Name, NodeId, Ident, FloatTy, IntTy, UintTy};
 use syntax::ext::base::SyntaxExtension;
-use syntax::ext::base::Determinacy::{self, Determined, Undetermined};
 use syntax::ext::base::MacroKind;
 use syntax::symbol::{Symbol, kw, sym};
 use syntax::util::lev_distance::find_best_match_for_name;
@@ -93,6 +93,18 @@ enum Weak {
     No,
 }
 
+#[derive(Copy, Clone, PartialEq, Debug)]
+pub enum Determinacy {
+    Determined,
+    Undetermined,
+}
+
+impl Determinacy {
+    fn determined(determined: bool) -> Determinacy {
+        if determined { Determinacy::Determined } else { Determinacy::Undetermined }
+    }
+}
+
 enum ScopeSet {
     Import(Namespace),
     AbsolutePath(Namespace),

src/librustc_resolve/macros.rs

@@ -1,4 +1,4 @@
-use crate::{AmbiguityError, AmbiguityKind, AmbiguityErrorMisc};
+use crate::{AmbiguityError, AmbiguityKind, AmbiguityErrorMisc, Determinacy};
 use crate::{CrateLint, Resolver, ResolutionError, ScopeSet, Weak};
 use crate::{Module, ModuleKind, NameBinding, NameBindingKind, PathResult, Segment, ToNameBinding};
 use crate::{is_known_tool, resolve_error};
@@ -14,7 +14,7 @@ use rustc::{ty, lint, span_bug};
 use syntax::ast::{self, Ident, ItemKind};
 use syntax::attr::{self, StabilityLevel};
 use syntax::errors::DiagnosticBuilder;
-use syntax::ext::base::{self, Determinacy};
+use syntax::ext::base::{self, Indeterminate};
 use syntax::ext::base::{MacroKind, SyntaxExtension};
 use syntax::ext::expand::{AstFragment, Invocation, InvocationKind};
 use syntax::ext::hygiene::{self, Mark};
@@ -216,7 +216,7 @@ impl<'a> base::Resolver for Resolver<'a> {
     }
 
     fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: Mark, force: bool)
-                                -> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
+                                -> Result<Option<Lrc<SyntaxExtension>>, Indeterminate> {
         let (path, kind, derives_in_scope, after_derive) = match invoc.kind {
             InvocationKind::Attr { attr: None, .. } =>
                 return Ok(None),
@@ -229,12 +229,7 @@ impl<'a> base::Resolver for Resolver<'a> {
         };
 
         let parent_scope = self.invoc_parent_scope(invoc_id, derives_in_scope);
-        let (res, ext) = match self.resolve_macro_to_res(path, kind, &parent_scope, true, force) {
-            Ok((res, ext)) => (res, ext),
-            // Return dummy syntax extensions for unresolved macros for better recovery.
-            Err(Determinacy::Determined) => (Res::Err, self.dummy_ext(kind)),
-            Err(Determinacy::Undetermined) => return Err(Determinacy::Undetermined),
-        };
+        let (res, ext) = self.resolve_macro_to_res(path, kind, &parent_scope, true, force)?;
 
         let span = invoc.span();
         let descr = fast_print_path(path);
@@ -287,7 +282,7 @@ impl<'a> Resolver<'a> {
         parent_scope: &ParentScope<'a>,
         trace: bool,
         force: bool,
-    ) -> Result<(Res, Lrc<SyntaxExtension>), Determinacy> {
+    ) -> Result<(Res, Lrc<SyntaxExtension>), Indeterminate> {
         let res = self.resolve_macro_to_res_inner(path, kind, parent_scope, trace, force);
 
         // Report errors and enforce feature gates for the resolved macro.
@@ -313,7 +308,14 @@ impl<'a> Resolver<'a> {
             }
         }
 
-        let res = res?;
+        let res = match res {
+            Err(Determinacy::Undetermined) => return Err(Indeterminate),
+            Ok(Res::Err) | Err(Determinacy::Determined) => {
+                // Return dummy syntax extensions for unresolved macros for better recovery.
+                return Ok((Res::Err, self.dummy_ext(kind)));
+            }
+            Ok(res) => res,
+        };
 
         match res {
             Res::Def(DefKind::Macro(_), def_id) => {
@@ -328,35 +330,23 @@ impl<'a> Resolver<'a> {
                 }
             }
             Res::NonMacroAttr(attr_kind) => {
-                if kind == MacroKind::Attr {
-                    if attr_kind == NonMacroAttrKind::Custom {
-                        assert!(path.segments.len() == 1);
-                        if !features.custom_attribute {
-                            let msg = format!("The attribute `{}` is currently unknown to the \
-                                               compiler and may have meaning added to it in the \
-                                               future", path);
-                            self.report_unknown_attribute(
-                                path.span,
-                                &path.segments[0].ident.as_str(),
-                                &msg,
-                                sym::custom_attribute,
-                            );
-                        }
+                if attr_kind == NonMacroAttrKind::Custom {
+                    assert!(path.segments.len() == 1);
+                    if !features.custom_attribute {
+                        let msg = format!("The attribute `{}` is currently unknown to the \
+                                            compiler and may have meaning added to it in the \
+                                            future", path);
+                        self.report_unknown_attribute(
+                            path.span,
+                            &path.segments[0].ident.as_str(),
+                            &msg,
+                            sym::custom_attribute,
+                        );
                     }
-                } else {
-                    // Not only attributes, but anything in macro namespace can result in
-                    // `Res::NonMacroAttr` definition (e.g., `inline!()`), so we must report
-                    // an error for those cases.
-                    let msg = format!("expected a macro, found {}", res.descr());
-                    self.session.span_err(path.span, &msg);
-                    return Err(Determinacy::Determined);
                 }
             }
-            Res::Err => {
-                return Err(Determinacy::Determined);
-            }
             _ => panic!("expected `DefKind::Macro` or `Res::NonMacroAttr`"),
-        }
+        };
 
         Ok((res, self.get_macro(res)))
     }
@@ -608,9 +598,7 @@ impl<'a> Resolver<'a> {
                                 result = Ok((binding, Flags::empty()));
                                 break;
                             }
-                            Err(Determinacy::Determined) => {}
-                            Err(Determinacy::Undetermined) =>
-                                result = Err(Determinacy::Undetermined),
+                            Err(Indeterminate) => result = Err(Determinacy::Undetermined),
                         }
                     }
                     result

src/librustc_resolve/resolve_imports.rs

@@ -2,6 +2,7 @@ use ImportDirectiveSubclass::*;
 
 use crate::{AmbiguityError, AmbiguityKind, AmbiguityErrorMisc};
 use crate::{CrateLint, Module, ModuleOrUniformRoot, PerNS, ScopeSet, Weak};
+use crate::Determinacy::{self, *};
 use crate::Namespace::{self, TypeNS, MacroNS};
 use crate::{NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError};
 use crate::{Resolver, Segment};
@@ -27,7 +28,6 @@ use rustc::util::nodemap::FxHashSet;
 use rustc::{bug, span_bug};
 
 use syntax::ast::{self, Ident, Name, NodeId, CRATE_NODE_ID};
-use syntax::ext::base::Determinacy::{self, Determined, Undetermined};
 use syntax::ext::hygiene::Mark;
 use syntax::symbol::kw;
 use syntax::util::lev_distance::find_best_match_for_name;

src/libsyntax/ext/base.rs

@@ -676,6 +676,9 @@ impl SyntaxExtension {
 
 pub type NamedSyntaxExtension = (Name, SyntaxExtension);
 
+/// Error type that denotes indeterminacy.
+pub struct Indeterminate;
+
 pub trait Resolver {
     fn next_node_id(&mut self) -> ast::NodeId;
 
@@ -689,23 +692,11 @@ pub trait Resolver {
     fn resolve_imports(&mut self);
 
     fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: Mark, force: bool)
-                                -> Result<Option<Lrc<SyntaxExtension>>, Determinacy>;
+                                -> Result<Option<Lrc<SyntaxExtension>>, Indeterminate>;
 
     fn check_unused_macros(&self);
 }
 
-#[derive(Copy, Clone, PartialEq, Debug)]
-pub enum Determinacy {
-    Determined,
-    Undetermined,
-}
-
-impl Determinacy {
-    pub fn determined(determined: bool) -> Determinacy {
-        if determined { Determinacy::Determined } else { Determinacy::Undetermined }
-    }
-}
-
 #[derive(Clone)]
 pub struct ModuleData {
     pub mod_path: Vec<ast::Ident>,

src/libsyntax/ext/expand.rs

@@ -313,9 +313,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
             let scope =
                 if self.monotonic { invoc.expansion_data.mark } else { orig_expansion_data.mark };
             let ext = match self.cx.resolver.resolve_macro_invocation(&invoc, scope, force) {
-                Ok(ext) => Some(ext),
-                Err(Determinacy::Determined) => None,
-                Err(Determinacy::Undetermined) => {
+                Ok(ext) => ext,
+                Err(Indeterminate) => {
                     undetermined_invocations.push(invoc);
                     continue
                 }
@@ -328,65 +327,61 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
             self.cx.current_expansion.mark = scope;
             // FIXME(jseyfried): Refactor out the following logic
             let (expanded_fragment, new_invocations) = if let Some(ext) = ext {
-                if let Some(ext) = ext {
-                    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() {
-                        let attr = attr::find_by_name(item.attrs(), sym::derive)
-                            .expect("`derive` attribute should exist");
-                        let span = attr.span;
-                        let mut err = self.cx.mut_span_err(span,
-                                                           "`derive` may only be applied to \
-                                                            structs, enums and unions");
-                        if let ast::AttrStyle::Inner = attr.style {
-                            let trait_list = traits.iter()
-                                .map(|t| t.to_string()).collect::<Vec<_>>();
-                            let suggestion = format!("#[derive({})]", trait_list.join(", "));
-                            err.span_suggestion(
-                                span, "try an outer attribute", suggestion,
-                                // We don't 𝑘𝑛𝑜𝑤 that the following item is an ADT
-                                Applicability::MaybeIncorrect
-                            );
-                        }
-                        err.emit();
+                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() {
+                    let attr = attr::find_by_name(item.attrs(), sym::derive)
+                        .expect("`derive` attribute should exist");
+                    let span = attr.span;
+                    let mut err = self.cx.mut_span_err(span,
+                                                        "`derive` may only be applied to \
+                                                        structs, enums and unions");
+                    if let ast::AttrStyle::Inner = attr.style {
+                        let trait_list = traits.iter()
+                            .map(|t| t.to_string()).collect::<Vec<_>>();
+                        let suggestion = format!("#[derive({})]", trait_list.join(", "));
+                        err.span_suggestion(
+                            span, "try an outer attribute", suggestion,
+                            // We don't 𝑘𝑛𝑜𝑤 that the following item is an ADT
+                            Applicability::MaybeIncorrect
+                        );
                     }
+                    err.emit();
+                }
 
-                    let mut item = self.fully_configure(item);
-                    item.visit_attrs(|attrs| attrs.retain(|a| a.path != sym::derive));
-                    let mut item_with_markers = item.clone();
-                    add_derived_markers(&mut self.cx, item.span(), &traits, &mut item_with_markers);
-                    let derives = derives.entry(invoc.expansion_data.mark).or_default();
-
-                    derives.reserve(traits.len());
-                    invocations.reserve(traits.len());
-                    for path in traits {
-                        let mark = Mark::fresh(self.cx.current_expansion.mark);
-                        derives.push(mark);
-                        invocations.push(Invocation {
-                            kind: InvocationKind::Derive {
-                                path,
-                                item: item.clone(),
-                                item_with_markers: item_with_markers.clone(),
-                            },
-                            fragment_kind: invoc.fragment_kind,
-                            expansion_data: ExpansionData {
-                                mark,
-                                ..invoc.expansion_data.clone()
-                            },
-                        });
-                    }
-                    let fragment = invoc.fragment_kind
-                        .expect_from_annotatables(::std::iter::once(item_with_markers));
-                    self.collect_invocations(fragment, derives)
-                } else {
-                    unreachable!()
+                let mut item = self.fully_configure(item);
+                item.visit_attrs(|attrs| attrs.retain(|a| a.path != sym::derive));
+                let mut item_with_markers = item.clone();
+                add_derived_markers(&mut self.cx, item.span(), &traits, &mut item_with_markers);
+                let derives = derives.entry(invoc.expansion_data.mark).or_default();
+
+                derives.reserve(traits.len());
+                invocations.reserve(traits.len());
+                for path in traits {
+                    let mark = Mark::fresh(self.cx.current_expansion.mark);
+                    derives.push(mark);
+                    invocations.push(Invocation {
+                        kind: InvocationKind::Derive {
+                            path,
+                            item: item.clone(),
+                            item_with_markers: item_with_markers.clone(),
+                        },
+                        fragment_kind: invoc.fragment_kind,
+                        expansion_data: ExpansionData {
+                            mark,
+                            ..invoc.expansion_data.clone()
+                        },
+                    });
                 }
+                let fragment = invoc.fragment_kind
+                    .expect_from_annotatables(::std::iter::once(item_with_markers));
+                self.collect_invocations(fragment, derives)
             } else {
-                self.collect_invocations(invoc.fragment_kind.dummy(invoc.span()).unwrap(), &[])
+                unreachable!()
             };
 
             if expanded_fragments.len() < depth {

src/test/ui/macros/macro-path-prelude-fail-4.rs

@@ -1,4 +1,4 @@
-#[derive(inline)] //~ ERROR expected a macro, found built-in attribute
+#[derive(inline)] //~ ERROR macro `inline` may not be used for derive attributes
 struct S;
 
 fn main() {}

src/test/ui/macros/macro-path-prelude-fail-4.stderr

@@ -1,4 +1,4 @@
-error: expected a macro, found built-in attribute
+error: macro `inline` may not be used for derive attributes
   --> $DIR/macro-path-prelude-fail-4.rs:1:10
    |
 LL | #[derive(inline)]

src/test/ui/tool-attributes/tool-attributes-misplaced-2.rs

@@ -1,6 +1,6 @@
-#[derive(rustfmt::skip)] //~ ERROR expected a macro, found tool attribute
+#[derive(rustfmt::skip)] //~ ERROR macro `rustfmt::skip` may not be used for derive attributes
 struct S;
 
 fn main() {
-    rustfmt::skip!(); //~ ERROR expected a macro, found tool attribute
+    rustfmt::skip!(); //~ ERROR `rustfmt::skip` can only be used in attributes
 }