resolve: More precise determinacy tracking during import/macro resolution

Author: petrochenkov

src/librustc_resolve/build_reduced_graph.rs

@@ -860,7 +860,6 @@ impl<'a, 'b, 'cl> BuildReducedGraphVisitor<'a, 'b, 'cl> {
         let invocation = self.resolver.invocations[&mark];
         invocation.module.set(self.resolver.current_module);
         invocation.parent_legacy_scope.set(self.current_legacy_scope);
-        invocation.output_legacy_scope.set(self.current_legacy_scope);
         invocation
     }
 }

src/librustc_resolve/lib.rs

@@ -97,6 +97,12 @@ fn is_known_tool(name: Name) -> bool {
     ["clippy", "rustfmt"].contains(&&*name.as_str())
 }
 
+enum DeterminacyExt {
+    Determined,
+    Undetermined,
+    WeakUndetermined,
+}
+
 /// A free importable items suggested in case of resolution failure.
 struct ImportSuggestion {
     path: Path,
@@ -1963,7 +1969,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 ModuleOrUniformRoot::Module(module),
                 ident,
                 ns,
-                false,
                 record_used,
                 path_span,
             );
@@ -1994,7 +1999,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 ModuleOrUniformRoot::Module(module),
                 ident,
                 ns,
-                false,
                 record_used,
                 path_span,
             );
@@ -2036,7 +2040,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                     ident,
                     ns,
                     false,
-                    false,
                     path_span,
                 ) {
                     return Some(LexicalScopeBinding::Item(binding));
@@ -2111,7 +2114,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             }
         }
         let result = self.resolve_ident_in_module_unadjusted(
-            module, ident, ns, false, record_used, span,
+            module, ident, ns, record_used, span,
         );
         self.current_module = orig_current_module;
         result
@@ -4334,7 +4337,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 ident,
                 ns,
                 false,
-                false,
                 module.span,
             ).is_ok() {
                 let import_id = match binding.kind {

src/librustc_resolve/macros.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 use {AmbiguityError, AmbiguityKind, AmbiguityErrorMisc};
-use {CrateLint, Resolver, ResolutionError, is_known_tool, resolve_error};
+use {CrateLint, DeterminacyExt, Resolver, ResolutionError, is_known_tool, resolve_error};
 use {Module, ModuleKind, NameBinding, NameBindingKind, PathResult, ToNameBinding};
 use ModuleOrUniformRoot;
 use Namespace::{self, *};
@@ -54,8 +54,8 @@ pub struct InvocationData<'a> {
     crate parent_legacy_scope: Cell<LegacyScope<'a>>,
     /// Legacy scope *produced* by expanding this macro invocation,
     /// includes all the macro_rules items, other invocations, etc generated by it.
-    /// Set to the parent scope if the macro is not expanded yet (as if the macro produced nothing).
-    crate output_legacy_scope: Cell<LegacyScope<'a>>,
+    /// `None` if the macro is not expanded yet.
+    crate output_legacy_scope: Cell<Option<LegacyScope<'a>>>,
 }
 
 impl<'a> InvocationData<'a> {
@@ -64,7 +64,7 @@ impl<'a> InvocationData<'a> {
             module: Cell::new(graph_root),
             def_index: CRATE_DEF_INDEX,
             parent_legacy_scope: Cell::new(LegacyScope::Empty),
-            output_legacy_scope: Cell::new(LegacyScope::Empty),
+            output_legacy_scope: Cell::new(Some(LegacyScope::Empty)),
         }
     }
 }
@@ -110,7 +110,7 @@ pub struct ParentScope<'a> {
 // Macro namespace is separated into two sub-namespaces, one for bang macros and
 // one for attribute-like macros (attributes, derives).
 // We ignore resolutions from one sub-namespace when searching names in scope for another.
-fn sub_namespace_mismatch(requirement: Option<MacroKind>, candidate: Option<MacroKind>) -> bool {
+fn sub_namespace_match(candidate: Option<MacroKind>, requirement: Option<MacroKind>) -> bool {
     #[derive(PartialEq)]
     enum SubNS { Bang, AttrLike }
     let sub_ns = |kind| match kind {
@@ -121,7 +121,7 @@ fn sub_namespace_mismatch(requirement: Option<MacroKind>, candidate: Option<Macr
     let requirement = requirement.and_then(|kind| sub_ns(kind));
     let candidate = candidate.and_then(|kind| sub_ns(kind));
     // "No specific sub-namespace" means "matches anything" for both requirements and candidates.
-    candidate.is_some() && requirement.is_some() && candidate != requirement
+    candidate.is_none() || requirement.is_none() || candidate == requirement
 }
 
 impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
@@ -136,7 +136,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
             module: Cell::new(module),
             def_index: module.def_id().unwrap().index,
             parent_legacy_scope: Cell::new(LegacyScope::Empty),
-            output_legacy_scope: Cell::new(LegacyScope::Empty),
+            output_legacy_scope: Cell::new(Some(LegacyScope::Empty)),
         }));
         mark
     }
@@ -212,7 +212,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
             expansion: mark,
         };
         fragment.visit_with(&mut visitor);
-        invocation.output_legacy_scope.set(visitor.current_legacy_scope);
+        invocation.output_legacy_scope.set(Some(visitor.current_legacy_scope));
     }
 
     fn add_builtin(&mut self, ident: ast::Ident, ext: Lrc<SyntaxExtension>) {
@@ -620,38 +620,50 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         let mut innermost_result: Option<(&NameBinding, Flags)> = None;
 
         // Go through all the scopes and try to resolve the name.
-        let mut where_to_resolve = WhereToResolve::DeriveHelpers;
+        let mut where_to_resolve = if ns == MacroNS {
+            WhereToResolve::DeriveHelpers
+        } else {
+            WhereToResolve::Module(parent_scope.module)
+        };
         let mut use_prelude = !parent_scope.module.no_implicit_prelude;
+        let mut determinacy = Determinacy::Determined;
         loop {
             let result = match where_to_resolve {
                 WhereToResolve::DeriveHelpers => {
                     let mut result = Err(Determinacy::Determined);
                     for derive in &parent_scope.derives {
                         let parent_scope = ParentScope { derives: Vec::new(), ..*parent_scope };
-                        if let Ok((_, ext)) = self.resolve_macro_to_def(derive, MacroKind::Derive,
-                                                                        &parent_scope, force) {
-                            if let SyntaxExtension::ProcMacroDerive(_, helper_attrs, _) = &*ext {
-                                if helper_attrs.contains(&ident.name) {
-                                    let binding =
-                                        (Def::NonMacroAttr(NonMacroAttrKind::DeriveHelper),
-                                        ty::Visibility::Public, derive.span, Mark::root())
-                                        .to_name_binding(self.arenas);
-                                    result = Ok((binding, Flags::empty()));
-                                    break;
+                        match self.resolve_macro_to_def(derive, MacroKind::Derive,
+                                                        &parent_scope, force) {
+                            Ok((_, ext)) => {
+                                if let SyntaxExtension::ProcMacroDerive(_, helpers, _) = &*ext {
+                                    if helpers.contains(&ident.name) {
+                                        let binding =
+                                            (Def::NonMacroAttr(NonMacroAttrKind::DeriveHelper),
+                                            ty::Visibility::Public, derive.span, Mark::root())
+                                            .to_name_binding(self.arenas);
+                                        result = Ok((binding, Flags::empty()));
+                                        break;
+                                    }
                                 }
                             }
+                            Err(Determinacy::Determined) => {}
+                            Err(Determinacy::Undetermined) =>
+                                result = Err(Determinacy::Undetermined),
                         }
                     }
                     result
                 }
                 WhereToResolve::MacroRules(legacy_scope) => match legacy_scope {
                     LegacyScope::Binding(legacy_binding) if ident == legacy_binding.ident =>
                         Ok((legacy_binding.binding, Flags::MACRO_RULES)),
+                    LegacyScope::Invocation(invoc) if invoc.output_legacy_scope.get().is_none() =>
+                        Err(Determinacy::Undetermined),
                     _ => Err(Determinacy::Determined),
                 }
                 WhereToResolve::Module(module) => {
                     let orig_current_module = mem::replace(&mut self.current_module, module);
-                    let binding = self.resolve_ident_in_module_unadjusted(
+                    let binding = self.resolve_ident_in_module_unadjusted_ext(
                         ModuleOrUniformRoot::Module(module),
                         ident,
                         ns,
@@ -660,17 +672,27 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                         path_span,
                     );
                     self.current_module = orig_current_module;
-                    let misc_flags = if module.is_normal() {
-                        Flags::MISC_SUGGEST_SELF
-                    } else {
-                        Flags::empty()
-                    };
-                    binding.map(|binding| (binding, Flags::MODULE | misc_flags))
+                    match binding {
+                        Ok(binding) => {
+                            let misc_flags = if module.is_normal() {
+                                Flags::MISC_SUGGEST_SELF
+                            } else {
+                                Flags::empty()
+                            };
+                            Ok((binding, Flags::MODULE | misc_flags))
+                        }
+                        Err(DeterminacyExt::Undetermined) =>
+                            return Err(Determinacy::determined(force)),
+                        Err(DeterminacyExt::WeakUndetermined) => Err(Determinacy::Undetermined),
+                        Err(DeterminacyExt::Determined) => Err(Determinacy::Determined),
+                    }
                 }
                 WhereToResolve::MacroUsePrelude => {
                     match self.macro_use_prelude.get(&ident.name).cloned() {
                         Some(binding) => Ok((binding, Flags::PRELUDE | Flags::MISC_FROM_PRELUDE)),
-                        None => Err(Determinacy::Determined),
+                        None => Err(Determinacy::determined(
+                            self.graph_root.unresolved_invocations.borrow().is_empty()
+                        ))
                     }
                 }
                 WhereToResolve::BuiltinMacros => {
@@ -701,14 +723,17 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     }
                 }
                 WhereToResolve::ExternPrelude => {
-                    let mut result = Err(Determinacy::Determined);
                     if use_prelude {
-                        if let Some(binding) = self.extern_prelude_get(ident, !record_used,
-                                                                       innermost_result.is_some()) {
-                            result = Ok((binding, Flags::PRELUDE));
+                        match self.extern_prelude_get(ident, !record_used,
+                                                      innermost_result.is_some()) {
+                            Some(binding) => Ok((binding, Flags::PRELUDE)),
+                            None => Err(Determinacy::determined(
+                                self.graph_root.unresolved_invocations.borrow().is_empty()
+                            )),
                         }
+                    } else {
+                        Err(Determinacy::Determined)
                     }
-                    result
                 }
                 WhereToResolve::ToolPrelude => {
                     if use_prelude && is_known_tool(ident.name) {
@@ -728,7 +753,6 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                                 ident,
                                 ns,
                                 false,
-                                false,
                                 path_span,
                             ) {
                                 result = Ok((binding, Flags::PRELUDE | Flags::MISC_FROM_PRELUDE));
@@ -749,54 +773,8 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                 }
             };
 
-            macro_rules! continue_search { () => {
-                where_to_resolve = match where_to_resolve {
-                    WhereToResolve::DeriveHelpers =>
-                        WhereToResolve::MacroRules(parent_scope.legacy),
-                    WhereToResolve::MacroRules(legacy_scope) => match legacy_scope {
-                        LegacyScope::Binding(binding) =>
-                            WhereToResolve::MacroRules(binding.parent_legacy_scope),
-                        LegacyScope::Invocation(invocation) =>
-                            WhereToResolve::MacroRules(invocation.output_legacy_scope.get()),
-                        LegacyScope::Empty => WhereToResolve::Module(parent_scope.module),
-                        LegacyScope::Uninitialized => unreachable!(),
-                    }
-                    WhereToResolve::Module(module) => {
-                        match self.hygienic_lexical_parent(module, &mut ident.span) {
-                            Some(parent_module) => WhereToResolve::Module(parent_module),
-                            None => {
-                                use_prelude = !module.no_implicit_prelude;
-                                match ns {
-                                    TypeNS => WhereToResolve::ExternPrelude,
-                                    ValueNS => WhereToResolve::StdLibPrelude,
-                                    MacroNS => WhereToResolve::MacroUsePrelude,
-                                }
-                            }
-                        }
-                    }
-                    WhereToResolve::MacroUsePrelude => WhereToResolve::BuiltinMacros,
-                    WhereToResolve::BuiltinMacros => WhereToResolve::BuiltinAttrs,
-                    WhereToResolve::BuiltinAttrs => WhereToResolve::LegacyPluginHelpers,
-                    WhereToResolve::LegacyPluginHelpers => break, // nowhere else to search
-                    WhereToResolve::ExternPrelude => WhereToResolve::ToolPrelude,
-                    WhereToResolve::ToolPrelude => WhereToResolve::StdLibPrelude,
-                    WhereToResolve::StdLibPrelude => match ns {
-                        TypeNS => WhereToResolve::BuiltinTypes,
-                        ValueNS => break, // nowhere else to search
-                        MacroNS => unreachable!(),
-                    }
-                    WhereToResolve::BuiltinTypes => break, // nowhere else to search
-                };
-
-                continue;
-            }}
-
             match result {
-                Ok((binding, flags)) => {
-                    if sub_namespace_mismatch(macro_kind, binding.macro_kind()) {
-                        continue_search!();
-                    }
-
+                Ok((binding, flags)) if sub_namespace_match(binding.macro_kind(), macro_kind) => {
                     if !record_used {
                         return Ok(binding);
                     }
@@ -857,22 +835,60 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                         // Found the first solution.
                         innermost_result = Some((binding, flags));
                     }
-
-                    continue_search!();
-                },
-                Err(Determinacy::Determined) => {
-                    continue_search!();
                 }
-                Err(Determinacy::Undetermined) => return Err(Determinacy::determined(force)),
+                Ok(..) | Err(Determinacy::Determined) => {}
+                Err(Determinacy::Undetermined) => determinacy = Determinacy::Undetermined
             }
+
+            where_to_resolve = match where_to_resolve {
+                WhereToResolve::DeriveHelpers =>
+                    WhereToResolve::MacroRules(parent_scope.legacy),
+                WhereToResolve::MacroRules(legacy_scope) => match legacy_scope {
+                    LegacyScope::Binding(binding) => WhereToResolve::MacroRules(
+                        binding.parent_legacy_scope
+                    ),
+                    LegacyScope::Invocation(invoc) => WhereToResolve::MacroRules(
+                        invoc.output_legacy_scope.get().unwrap_or(invoc.parent_legacy_scope.get())
+                    ),
+                    LegacyScope::Empty => WhereToResolve::Module(parent_scope.module),
+                    LegacyScope::Uninitialized => unreachable!(),
+                }
+                WhereToResolve::Module(module) => {
+                    match self.hygienic_lexical_parent(module, &mut ident.span) {
+                        Some(parent_module) => WhereToResolve::Module(parent_module),
+                        None => {
+                            use_prelude = !module.no_implicit_prelude;
+                            match ns {
+                                TypeNS => WhereToResolve::ExternPrelude,
+                                ValueNS => WhereToResolve::StdLibPrelude,
+                                MacroNS => WhereToResolve::MacroUsePrelude,
+                            }
+                        }
+                    }
+                }
+                WhereToResolve::MacroUsePrelude => WhereToResolve::BuiltinMacros,
+                WhereToResolve::BuiltinMacros => WhereToResolve::BuiltinAttrs,
+                WhereToResolve::BuiltinAttrs => WhereToResolve::LegacyPluginHelpers,
+                WhereToResolve::LegacyPluginHelpers => break, // nowhere else to search
+                WhereToResolve::ExternPrelude => WhereToResolve::ToolPrelude,
+                WhereToResolve::ToolPrelude => WhereToResolve::StdLibPrelude,
+                WhereToResolve::StdLibPrelude => match ns {
+                    TypeNS => WhereToResolve::BuiltinTypes,
+                    ValueNS => break, // nowhere else to search
+                    MacroNS => unreachable!(),
+                }
+                WhereToResolve::BuiltinTypes => break, // nowhere else to search
+            };
+
+            continue;
         }
 
         // The first found solution was the only one, return it.
         if let Some((binding, ..)) = innermost_result {
             return Ok(binding);
         }
 
-        let determinacy = Determinacy::determined(force);
+        let determinacy = Determinacy::determined(determinacy == Determinacy::Determined || force);
         if determinacy == Determinacy::Determined && macro_kind == Some(MacroKind::Attr) {
             // For single-segment attributes interpret determinate "no resolution" as a custom
             // attribute. (Lexical resolution implies the first segment and attr kind should imply
@@ -1016,7 +1032,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     def_index: invoc.def_index,
                     module: Cell::new(graph_root),
                     parent_legacy_scope: Cell::new(LegacyScope::Uninitialized),
-                    output_legacy_scope: Cell::new(LegacyScope::Uninitialized),
+                    output_legacy_scope: Cell::new(None),
                 })
             });
         };