7 files changed, 193 insertions, 96 deletions

diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs
index 9a432f7d1..ff4b4a0cf 100644
--- a/crates/hir_def/src/body.rs
+++ b/crates/hir_def/src/body.rs
@@ -123,7 +123,7 @@ impl Expander {
             Some(it) => it,
             None => {
                 if err.is_none() {
-                    eprintln!("no error despite `as_call_id_with_errors` returning `None`");
+                    log::warn!("no error despite `as_call_id_with_errors` returning `None`");
                 }
                 return ExpandResult { value: None, err };
             }
diff --git a/crates/hir_def/src/body/tests/block.rs b/crates/hir_def/src/body/tests/block.rs
index a5ec0883f..8bca72a17 100644
--- a/crates/hir_def/src/body/tests/block.rs
+++ b/crates/hir_def/src/body/tests/block.rs
@@ -259,3 +259,32 @@ fn main() {
     "#]],
     );
 }
+
+#[test]
+fn underscore_import() {
+    // This used to panic, because the default (private) visibility inside block expressions would
+    // point into the containing `DefMap`, which visibilities should never be able to do.
+    mark::check!(adjust_vis_in_block_def_map);
+    check_at(
+        r#"
+mod m {
+    fn main() {
+        use Tr as _;
+        trait Tr {}
+        $0
+    }
+}
+    "#,
+        expect![[r#"
+        block scope
+        _: t
+        Tr: t
+
+        crate
+        m: t
+
+        crate::m
+        main: v
+    "#]],
+    );
+}
diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs
index ab3f059ce..ac7474f63 100644
--- a/crates/hir_def/src/diagnostics.rs
+++ b/crates/hir_def/src/diagnostics.rs
@@ -95,6 +95,34 @@ impl Diagnostic for UnresolvedImport {
     }
 }
 
+// Diagnostic: unresolved-macro-call
+//
+// This diagnostic is triggered if rust-analyzer is unable to resolove path to a
+// macro in a macro invocation.
+#[derive(Debug)]
+pub struct UnresolvedMacroCall {
+    pub file: HirFileId,
+    pub node: AstPtr<ast::MacroCall>,
+}
+
+impl Diagnostic for UnresolvedMacroCall {
+    fn code(&self) -> DiagnosticCode {
+        DiagnosticCode("unresolved-macro-call")
+    }
+    fn message(&self) -> String {
+        "unresolved macro call".to_string()
+    }
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
+        InFile::new(self.file, self.node.clone().into())
+    }
+    fn as_any(&self) -> &(dyn Any + Send + 'static) {
+        self
+    }
+    fn is_experimental(&self) -> bool {
+        true
+    }
+}
+
 // Diagnostic: inactive-code
 //
 // This diagnostic is shown for code with inactive `#[cfg]` attributes.
diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs
index b50923747..6802bc250 100644
--- a/crates/hir_def/src/lib.rs
+++ b/crates/hir_def/src/lib.rs
@@ -57,8 +57,10 @@ use std::{
 
 use base_db::{impl_intern_key, salsa, CrateId};
 use hir_expand::{
-    ast_id_map::FileAstId, eager::expand_eager_macro, hygiene::Hygiene, AstId, HirFileId, InFile,
-    MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
+    ast_id_map::FileAstId,
+    eager::{expand_eager_macro, ErrorEmitted},
+    hygiene::Hygiene,
+    AstId, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
 };
 use la_arena::Idx;
 use nameres::DefMap;
@@ -592,8 +594,15 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
             error_sink(mbe::ExpandError::Other("malformed macro invocation".into()));
         }
 
-        AstIdWithPath::new(ast_id.file_id, ast_id.value, path?)
-            .as_call_id_with_errors(db, krate, resolver, error_sink)
+        macro_call_as_call_id(
+            &AstIdWithPath::new(ast_id.file_id, ast_id.value, path?),
+            db,
+            krate,
+            resolver,
+            error_sink,
+        )
+        .ok()?
+        .ok()
     }
 }
 
@@ -610,61 +619,50 @@ impl<T: ast::AstNode> AstIdWithPath<T> {
     }
 }
 
-impl AsMacroCall for AstIdWithPath<ast::MacroCall> {
-    fn as_call_id_with_errors(
-        &self,
-        db: &dyn db::DefDatabase,
-        krate: CrateId,
-        resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
-        error_sink: &mut dyn FnMut(mbe::ExpandError),
-    ) -> Option<MacroCallId> {
-        let def: MacroDefId = resolver(self.path.clone()).or_else(|| {
-            error_sink(mbe::ExpandError::Other(format!("could not resolve macro `{}`", self.path)));
-            None
-        })?;
-
-        if let MacroDefKind::BuiltInEager(_) = def.kind {
-            let macro_call = InFile::new(self.ast_id.file_id, self.ast_id.to_node(db.upcast()));
-            let hygiene = Hygiene::new(db.upcast(), self.ast_id.file_id);
-
-            Some(
-                expand_eager_macro(
-                    db.upcast(),
-                    krate,
-                    macro_call,
-                    def,
-                    &|path: ast::Path| resolver(path::ModPath::from_src(path, &hygiene)?),
-                    error_sink,
-                )
-                .ok()?
-                .into(),
-            )
-        } else {
-            Some(def.as_lazy_macro(db.upcast(), krate, MacroCallKind::FnLike(self.ast_id)).into())
-        }
-    }
+struct UnresolvedMacro;
+
+fn macro_call_as_call_id(
+    call: &AstIdWithPath<ast::MacroCall>,
+    db: &dyn db::DefDatabase,
+    krate: CrateId,
+    resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
+    error_sink: &mut dyn FnMut(mbe::ExpandError),
+) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
+    let def: MacroDefId = resolver(call.path.clone()).ok_or(UnresolvedMacro)?;
+
+    let res = if let MacroDefKind::BuiltInEager(_) = def.kind {
+        let macro_call = InFile::new(call.ast_id.file_id, call.ast_id.to_node(db.upcast()));
+        let hygiene = Hygiene::new(db.upcast(), call.ast_id.file_id);
+
+        expand_eager_macro(
+            db.upcast(),
+            krate,
+            macro_call,
+            def,
+            &|path: ast::Path| resolver(path::ModPath::from_src(path, &hygiene)?),
+            error_sink,
+        )
+        .map(MacroCallId::from)
+    } else {
+        Ok(def.as_lazy_macro(db.upcast(), krate, MacroCallKind::FnLike(call.ast_id)).into())
+    };
+    Ok(res)
 }
 
-impl AsMacroCall for AstIdWithPath<ast::Item> {
-    fn as_call_id_with_errors(
-        &self,
-        db: &dyn db::DefDatabase,
-        krate: CrateId,
-        resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
-        error_sink: &mut dyn FnMut(mbe::ExpandError),
-    ) -> Option<MacroCallId> {
-        let def: MacroDefId = resolver(self.path.clone()).or_else(|| {
-            error_sink(mbe::ExpandError::Other(format!("could not resolve macro `{}`", self.path)));
-            None
-        })?;
-
-        Some(
-            def.as_lazy_macro(
-                db.upcast(),
-                krate,
-                MacroCallKind::Attr(self.ast_id, self.path.segments().last()?.to_string()),
-            )
-            .into(),
+fn item_attr_as_call_id(
+    item_attr: &AstIdWithPath<ast::Item>,
+    db: &dyn db::DefDatabase,
+    krate: CrateId,
+    resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
+) -> Result<MacroCallId, UnresolvedMacro> {
+    let def: MacroDefId = resolver(item_attr.path.clone()).ok_or(UnresolvedMacro)?;
+    let last_segment = item_attr.path.segments().last().ok_or(UnresolvedMacro)?;
+    let res = def
+        .as_lazy_macro(
+            db.upcast(),
+            krate,
+            MacroCallKind::Attr(item_attr.ast_id, last_segment.to_string()),
         )
-    }
+        .into();
+    Ok(res)
 }
diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs
index f92232eb3..6a3456f2e 100644
--- a/crates/hir_def/src/nameres.rs
+++ b/crates/hir_def/src/nameres.rs
@@ -417,6 +417,8 @@ mod diagnostics {
 
         UnresolvedProcMacro { ast: MacroCallKind },
 
+        UnresolvedMacroCall { ast: AstId<ast::MacroCall> },
+
         MacroError { ast: MacroCallKind, message: String },
     }
 
@@ -477,6 +479,13 @@ mod diagnostics {
             Self { in_module: container, kind: DiagnosticKind::MacroError { ast, message } }
         }
 
+        pub(super) fn unresolved_macro_call(
+            container: LocalModuleId,
+            ast: AstId<ast::MacroCall>,
+        ) -> Self {
+            Self { in_module: container, kind: DiagnosticKind::UnresolvedMacroCall { ast } }
+        }
+
         pub(super) fn add_to(
             &self,
             db: &dyn DefDatabase,
@@ -589,6 +598,11 @@ mod diagnostics {
                     });
                 }
 
+                DiagnosticKind::UnresolvedMacroCall { ast } => {
+                    let node = ast.to_node(db.upcast());
+                    sink.push(UnresolvedMacroCall { file: ast.file_id, node: AstPtr::new(&node) });
+                }
+
                 DiagnosticKind::MacroError { ast, message } => {
                     let (file, ast) = match ast {
                         MacroCallKind::FnLike(ast) => {
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index 9996a0807..e51d89b43 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -13,7 +13,7 @@ use hir_expand::{
     builtin_macro::find_builtin_macro,
     name::{AsName, Name},
     proc_macro::ProcMacroExpander,
-    HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
+    HirFileId, MacroCallId, MacroDefId, MacroDefKind,
 };
 use hir_expand::{InFile, MacroCallLoc};
 use rustc_hash::{FxHashMap, FxHashSet};
@@ -24,11 +24,13 @@ use tt::{Leaf, TokenTree};
 use crate::{
     attr::Attrs,
     db::DefDatabase,
+    item_attr_as_call_id,
     item_scope::{ImportType, PerNsGlobImports},
     item_tree::{
         self, FileItemTreeId, ItemTree, ItemTreeId, MacroCall, MacroRules, Mod, ModItem, ModKind,
         StructDefKind,
     },
+    macro_call_as_call_id,
     nameres::{
         diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint,
         BuiltinShadowMode, DefMap, ModuleData, ModuleOrigin, ResolveMode,
@@ -36,9 +38,9 @@ use crate::{
     path::{ImportAlias, ModPath, PathKind},
     per_ns::PerNs,
     visibility::{RawVisibility, Visibility},
-    AdtId, AsMacroCall, AstId, AstIdWithPath, ConstLoc, ContainerId, EnumLoc, EnumVariantId,
-    FunctionLoc, ImplLoc, Intern, LocalModuleId, ModuleDefId, StaticLoc, StructLoc, TraitLoc,
-    TypeAliasLoc, UnionLoc,
+    AdtId, AstId, AstIdWithPath, ConstLoc, ContainerId, EnumLoc, EnumVariantId, FunctionLoc,
+    ImplLoc, Intern, LocalModuleId, ModuleDefId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc,
+    UnionLoc, UnresolvedMacro,
 };
 
 const GLOB_RECURSION_LIMIT: usize = 100;
@@ -790,8 +792,11 @@ impl DefCollector<'_> {
                 return false;
             }
 
-            if let Some(call_id) =
-                directive.ast_id.as_call_id(self.db, self.def_map.krate, |path| {
+            match macro_call_as_call_id(
+                &directive.ast_id,
+                self.db,
+                self.def_map.krate,
+                |path| {
                     let resolved_res = self.def_map.resolve_path_fp_with_macro(
                         self.db,
                         ResolveMode::Other,
@@ -800,24 +805,29 @@ impl DefCollector<'_> {
                         BuiltinShadowMode::Module,
                     );
                     resolved_res.resolved_def.take_macros()
-                })
-            {
-                resolved.push((directive.module_id, call_id, directive.depth));
-                res = ReachedFixedPoint::No;
-                return false;
+                },
+                &mut |_err| (),
+            ) {
+                Ok(Ok(call_id)) => {
+                    resolved.push((directive.module_id, call_id, directive.depth));
+                    res = ReachedFixedPoint::No;
+                    return false;
+                }
+                Err(UnresolvedMacro) | Ok(Err(_)) => {}
             }
 
             true
         });
         attribute_macros.retain(|directive| {
-            if let Some(call_id) =
-                directive.ast_id.as_call_id(self.db, self.def_map.krate, |path| {
-                    self.resolve_attribute_macro(&directive, &path)
-                })
-            {
-                resolved.push((directive.module_id, call_id, 0));
-                res = ReachedFixedPoint::No;
-                return false;
+            match item_attr_as_call_id(&directive.ast_id, self.db, self.def_map.krate, |path| {
+                self.resolve_attribute_macro(&directive, &path)
+            }) {
+                Ok(call_id) => {
+                    resolved.push((directive.module_id, call_id, 0));
+                    res = ReachedFixedPoint::No;
+                    return false;
+                }
+                Err(UnresolvedMacro) => (),
             }
 
             true
@@ -902,7 +912,8 @@ impl DefCollector<'_> {
 
         for directive in &self.unexpanded_macros {
             let mut error = None;
-            directive.ast_id.as_call_id_with_errors(
+            match macro_call_as_call_id(
+                &directive.ast_id,
                 self.db,
                 self.def_map.krate,
                 |path| {
@@ -918,15 +929,15 @@ impl DefCollector<'_> {
                 &mut |e| {
                     error.get_or_insert(e);
                 },
-            );
-
-            if let Some(err) = error {
-                self.def_map.diagnostics.push(DefDiagnostic::macro_error(
-                    directive.module_id,
-                    MacroCallKind::FnLike(directive.ast_id.ast_id),
-                    err.to_string(),
-                ));
-            }
+            ) {
+                Ok(_) => (),
+                Err(UnresolvedMacro) => {
+                    self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
+                        directive.module_id,
+                        directive.ast_id.ast_id,
+                    ));
+                }
+            };
         }
 
         // Emit diagnostics for all remaining unresolved imports.
@@ -1446,8 +1457,11 @@ impl ModCollector<'_, '_> {
         let mut ast_id = AstIdWithPath::new(self.file_id, mac.ast_id, mac.path.clone());
 
         // Case 1: try to resolve in legacy scope and expand macro_rules
-        if let Some(macro_call_id) =
-            ast_id.as_call_id(self.def_collector.db, self.def_collector.def_map.krate, |path| {
+        if let Ok(Ok(macro_call_id)) = macro_call_as_call_id(
+            &ast_id,
+            self.def_collector.db,
+            self.def_collector.def_map.krate,
+            |path| {
                 path.as_ident().and_then(|name| {
                     self.def_collector.def_map.with_ancestor_maps(
                         self.def_collector.db,
@@ -1455,8 +1469,9 @@ impl ModCollector<'_, '_> {
                         &mut |map, module| map[module].scope.get_legacy_macro(&name),
                     )
                 })
-            })
-        {
+            },
+            &mut |_err| (),
+        ) {
             self.def_collector.unexpanded_macros.push(MacroDirective {
                 module_id: self.module_id,
                 ast_id,
diff --git a/crates/hir_def/src/nameres/path_resolution.rs b/crates/hir_def/src/nameres/path_resolution.rs
index fdcdc23ae..dd1db0094 100644
--- a/crates/hir_def/src/nameres/path_resolution.rs
+++ b/crates/hir_def/src/nameres/path_resolution.rs
@@ -77,7 +77,7 @@ impl DefMap {
         original_module: LocalModuleId,
         visibility: &RawVisibility,
     ) -> Option<Visibility> {
-        match visibility {
+        let mut vis = match visibility {
             RawVisibility::Module(path) => {
                 let (result, remaining) =
                     self.resolve_path(db, original_module, &path, BuiltinShadowMode::Module);
@@ -86,15 +86,28 @@ impl DefMap {
                 }
                 let types = result.take_types()?;
                 match types {
-                    ModuleDefId::ModuleId(m) => Some(Visibility::Module(m)),
+                    ModuleDefId::ModuleId(m) => Visibility::Module(m),
                     _ => {
                         // error: visibility needs to refer to module
-                        None
+                        return None;
                     }
                 }
             }
-            RawVisibility::Public => Some(Visibility::Public),
+            RawVisibility::Public => Visibility::Public,
+        };
+
+        // In block expressions, `self` normally refers to the containing non-block module, and
+        // `super` to its parent (etc.). However, visibilities must only refer to a module in the
+        // DefMap they're written in, so we restrict them when that happens.
+        if let Visibility::Module(m) = vis {
+            if self.block_id() != m.block {
+                mark::hit!(adjust_vis_in_block_def_map);
+                vis = Visibility::Module(self.module_id(self.root()));
+                log::debug!("visibility {:?} points outside DefMap, adjusting to {:?}", m, vis);
+            }
         }
+
+        Some(vis)
     }
 
     // Returns Yes if we are sure that additions to `ItemMap` wouldn't change