From c80c34867fab85bfa0ac3661e8abd264991af324 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 24 Jun 2022 13:03:13 +0200 Subject: [PATCH 1/2] Improve proc macro errors a bit Distinguish between - there is no build data (for some reason?) - there is build data, but the cargo package didn't build a proc macro dylib - there is a proc macro dylib, but it didn't contain the proc macro we expected - the name did not resolve to any macro (this is now an unresolved_macro_call even for attributes) I changed the handling of disabled attribute macro expansion to immediately ignore the macro and report an unresolved_proc_macro, because otherwise they would now result in loud unresolved_macro_call errors. I hope this doesn't break anything. Also try to improve error ranges for unresolved_macro_call / macro_error by reusing the code for unresolved_proc_macro. It's not perfect but probably better than before. --- Cargo.lock | 1 + crates/hir-def/src/nameres/collector.rs | 17 +- crates/hir-expand/Cargo.toml | 1 + crates/hir-expand/src/proc_macro.rs | 17 +- crates/hir/src/diagnostics.rs | 2 + crates/hir/src/lib.rs | 162 +++++++++--------- .../src/handlers/macro_error.rs | 32 ++-- .../src/handlers/unresolved_macro_call.rs | 22 +-- crates/project-model/src/tests.rs | 64 +++---- crates/project-model/src/workspace.rs | 9 +- 10 files changed, 164 insertions(+), 163 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 49a73d60f56..22292cdcc67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -538,6 +538,7 @@ dependencies = [ "mbe", "profile", "rustc-hash", + "stdx", "syntax", "tracing", "tt", diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 1f095846b1c..09594d1e616 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -436,7 +436,7 @@ impl DefCollector<'_> { let mut unresolved_macros = mem::take(&mut self.unresolved_macros); let pos = unresolved_macros.iter().position(|directive| { if let MacroDirectiveKind::Attr { ast_id, mod_item, attr, tree } = &directive.kind { - self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( + self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call( directive.module_id, MacroCallKind::Attr { ast_id: ast_id.ast_id, @@ -444,7 +444,7 @@ impl DefCollector<'_> { invoc_attr_index: attr.id.ast_index, is_derive: false, }, - None, + attr.path().clone(), )); self.skip_attrs.insert(ast_id.ast_id.with_value(*mod_item), attr.id); @@ -1218,10 +1218,6 @@ impl DefCollector<'_> { return recollect_without(self); } - if !self.db.enable_proc_attr_macros() { - return true; - } - // Not resolved to a derive helper or the derive attribute, so try to treat as a normal attribute. let call_id = attr_macro_as_call_id( self.db, @@ -1233,6 +1229,15 @@ impl DefCollector<'_> { ); let loc: MacroCallLoc = self.db.lookup_intern_macro_call(call_id); + if !self.db.enable_proc_attr_macros() { + self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( + directive.module_id, + loc.kind, + Some(loc.def.krate), + )); + return recollect_without(self); + } + // Skip #[test]/#[bench] expansion, which would merely result in more memory usage // due to duplicating functions into macro expansions if matches!( diff --git a/crates/hir-expand/Cargo.toml b/crates/hir-expand/Cargo.toml index 9e72f27ea4e..0e6fad16103 100644 --- a/crates/hir-expand/Cargo.toml +++ b/crates/hir-expand/Cargo.toml @@ -20,6 +20,7 @@ hashbrown = { version = "0.12.1", features = [ "inline-more", ], default-features = false } +stdx = { path = "../stdx", version = "0.0.0" } base-db = { path = "../base-db", version = "0.0.0" } cfg = { path = "../cfg", version = "0.0.0" } syntax = { path = "../syntax", version = "0.0.0" } diff --git a/crates/hir-expand/src/proc_macro.rs b/crates/hir-expand/src/proc_macro.rs index b2686592a55..29d78e21ba3 100644 --- a/crates/hir-expand/src/proc_macro.rs +++ b/crates/hir-expand/src/proc_macro.rs @@ -1,6 +1,7 @@ //! Proc Macro Expander stub use base_db::{CrateId, ProcMacroExpansionError, ProcMacroId, ProcMacroKind}; +use stdx::never; use crate::{db::AstDatabase, ExpandError, ExpandResult}; @@ -36,18 +37,20 @@ impl ProcMacroExpander { let krate_graph = db.crate_graph(); let proc_macros = match &krate_graph[self.krate].proc_macro { Ok(proc_macros) => proc_macros, - Err(e) => { - return ExpandResult::only_err(ExpandError::Other( - e.clone().into_boxed_str(), - )) + Err(_) => { + never!("Non-dummy expander even though there are no proc macros"); + return ExpandResult::only_err(ExpandError::Other("Internal error".into())); } }; let proc_macro = match proc_macros.get(id.0 as usize) { Some(proc_macro) => proc_macro, None => { - return ExpandResult::only_err(ExpandError::Other( - "No proc-macro found.".into(), - )) + never!( + "Proc macro index out of bounds: the length is {} but the index is {}", + proc_macros.len(), + id.0 + ); + return ExpandResult::only_err(ExpandError::Other("Internal error".into())); } }; diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 1f65c05c1ea..6a06c6b10f5 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -69,6 +69,7 @@ pub struct UnresolvedImport { #[derive(Debug, Clone, Eq, PartialEq)] pub struct UnresolvedMacroCall { pub macro_call: InFile, + pub precise_location: Option, pub path: ModPath, pub is_bang: bool, } @@ -95,6 +96,7 @@ pub struct UnresolvedProcMacro { #[derive(Debug, Clone, Eq, PartialEq)] pub struct MacroError { pub node: InFile, + pub precise_location: Option, pub message: String, } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index cc4d14c12da..7a31a8a4173 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -73,7 +73,7 @@ use rustc_hash::FxHashSet; use stdx::{format_to, impl_from, never}; use syntax::{ ast::{self, HasAttrs as _, HasDocComments, HasName}, - AstNode, AstPtr, SmolStr, SyntaxNodePtr, T, + AstNode, AstPtr, SmolStr, SyntaxNodePtr, TextRange, T, }; use crate::db::{DefDatabase, HirDatabase}; @@ -628,67 +628,7 @@ fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec, diag: } DefDiagnosticKind::UnresolvedProcMacro { ast, krate } => { - let (node, precise_location, macro_name, kind) = match ast { - MacroCallKind::FnLike { ast_id, .. } => { - let node = ast_id.to_node(db.upcast()); - ( - ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))), - node.path().map(|it| it.syntax().text_range()), - node.path().and_then(|it| it.segment()).map(|it| it.to_string()), - MacroKind::ProcMacro, - ) - } - MacroCallKind::Derive { ast_id, derive_attr_index, derive_index } => { - let node = ast_id.to_node(db.upcast()); - // Compute the precise location of the macro name's token in the derive - // list. - let token = (|| { - let derive_attr = node - .doc_comments_and_attrs() - .nth(*derive_attr_index as usize) - .and_then(Either::left)?; - let token_tree = derive_attr.meta()?.token_tree()?; - let group_by = token_tree - .syntax() - .children_with_tokens() - .filter_map(|elem| match elem { - syntax::NodeOrToken::Token(tok) => Some(tok), - _ => None, - }) - .group_by(|t| t.kind() == T![,]); - let (_, mut group) = group_by - .into_iter() - .filter(|&(comma, _)| !comma) - .nth(*derive_index as usize)?; - group.find(|t| t.kind() == T![ident]) - })(); - ( - ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))), - token.as_ref().map(|tok| tok.text_range()), - token.as_ref().map(ToString::to_string), - MacroKind::Derive, - ) - } - MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => { - let node = ast_id.to_node(db.upcast()); - let attr = node - .doc_comments_and_attrs() - .nth((*invoc_attr_index) as usize) - .and_then(Either::left) - .unwrap_or_else(|| panic!("cannot find attribute #{}", invoc_attr_index)); - - ( - ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&attr))), - Some(attr.syntax().text_range()), - attr.path() - .and_then(|path| path.segment()) - .and_then(|seg| seg.name_ref()) - .as_ref() - .map(ToString::to_string), - MacroKind::Attr, - ) - } - }; + let (node, precise_location, macro_name, kind) = precise_macro_call_location(ast, db); acc.push( UnresolvedProcMacro { node, precise_location, macro_name, kind, krate: *krate } .into(), @@ -696,10 +636,11 @@ fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec, diag: } DefDiagnosticKind::UnresolvedMacroCall { ast, path } => { - let node = ast.to_node(db.upcast()); + let (node, precise_location, _, _) = precise_macro_call_location(ast, db); acc.push( UnresolvedMacroCall { - macro_call: InFile::new(node.file_id, SyntaxNodePtr::new(&node.value)), + macro_call: node, + precise_location, path: path.clone(), is_bang: matches!(ast, MacroCallKind::FnLike { .. }), } @@ -708,23 +649,8 @@ fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec, diag: } DefDiagnosticKind::MacroError { ast, message } => { - let node = match ast { - MacroCallKind::FnLike { ast_id, .. } => { - let node = ast_id.to_node(db.upcast()); - ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))) - } - MacroCallKind::Derive { ast_id, .. } => { - // FIXME: point to the attribute instead, this creates very large diagnostics - let node = ast_id.to_node(db.upcast()); - ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))) - } - MacroCallKind::Attr { ast_id, .. } => { - // FIXME: point to the attribute instead, this creates very large diagnostics - let node = ast_id.to_node(db.upcast()); - ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))) - } - }; - acc.push(MacroError { node, message: message.clone() }.into()); + let (node, precise_location, _, _) = precise_macro_call_location(ast, db); + acc.push(MacroError { node, precise_location, message: message.clone() }.into()); } DefDiagnosticKind::UnimplementedBuiltinMacro { ast } => { @@ -771,6 +697,78 @@ fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec, diag: } } +fn precise_macro_call_location( + ast: &MacroCallKind, + db: &dyn HirDatabase, +) -> (InFile, Option, Option, MacroKind) { + // FIXME: maaybe we actually want slightly different ranges for the different macro diagnostics + // - e.g. the full attribute for macro errors, but only the name for name resolution + match ast { + MacroCallKind::FnLike { ast_id, .. } => { + let node = ast_id.to_node(db.upcast()); + ( + ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))), + node.path() + .and_then(|it| it.segment()) + .and_then(|it| it.name_ref()) + .map(|it| it.syntax().text_range()), + node.path().and_then(|it| it.segment()).map(|it| it.to_string()), + MacroKind::ProcMacro, + ) + } + MacroCallKind::Derive { ast_id, derive_attr_index, derive_index } => { + let node = ast_id.to_node(db.upcast()); + // Compute the precise location of the macro name's token in the derive + // list. + let token = (|| { + let derive_attr = node + .doc_comments_and_attrs() + .nth(*derive_attr_index as usize) + .and_then(Either::left)?; + let token_tree = derive_attr.meta()?.token_tree()?; + let group_by = token_tree + .syntax() + .children_with_tokens() + .filter_map(|elem| match elem { + syntax::NodeOrToken::Token(tok) => Some(tok), + _ => None, + }) + .group_by(|t| t.kind() == T![,]); + let (_, mut group) = group_by + .into_iter() + .filter(|&(comma, _)| !comma) + .nth(*derive_index as usize)?; + group.find(|t| t.kind() == T![ident]) + })(); + ( + ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))), + token.as_ref().map(|tok| tok.text_range()), + token.as_ref().map(ToString::to_string), + MacroKind::Derive, + ) + } + MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => { + let node = ast_id.to_node(db.upcast()); + let attr = node + .doc_comments_and_attrs() + .nth((*invoc_attr_index) as usize) + .and_then(Either::left) + .unwrap_or_else(|| panic!("cannot find attribute #{}", invoc_attr_index)); + + ( + ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&attr))), + Some(attr.syntax().text_range()), + attr.path() + .and_then(|path| path.segment()) + .and_then(|seg| seg.name_ref()) + .as_ref() + .map(ToString::to_string), + MacroKind::Attr, + ) + } + } +} + impl HasVisibility for Module { fn visibility(&self, db: &dyn HirDatabase) -> Visibility { let def_map = self.id.def_map(db.upcast()); @@ -1156,6 +1154,7 @@ impl DefWithBody { BodyDiagnostic::MacroError { node, message } => acc.push( MacroError { node: node.clone().map(|it| it.into()), + precise_location: None, message: message.to_string(), } .into(), @@ -1173,6 +1172,7 @@ impl DefWithBody { BodyDiagnostic::UnresolvedMacroCall { node, path } => acc.push( UnresolvedMacroCall { macro_call: node.clone().map(|ast_ptr| ast_ptr.into()), + precise_location: None, path: path.clone(), is_bang: true, } diff --git a/crates/ide-diagnostics/src/handlers/macro_error.rs b/crates/ide-diagnostics/src/handlers/macro_error.rs index 53d0131e02d..83f2bc2237b 100644 --- a/crates/ide-diagnostics/src/handlers/macro_error.rs +++ b/crates/ide-diagnostics/src/handlers/macro_error.rs @@ -4,12 +4,12 @@ use crate::{Diagnostic, DiagnosticsContext}; // // This diagnostic is shown for macro expansion errors. pub(crate) fn macro_error(ctx: &DiagnosticsContext<'_>, d: &hir::MacroError) -> Diagnostic { - Diagnostic::new( - "macro-error", - d.message.clone(), - ctx.sema.diagnostics_display_range(d.node.clone()).range, - ) - .experimental() + // Use more accurate position if available. + let display_range = d + .precise_location + .unwrap_or_else(|| ctx.sema.diagnostics_display_range(d.node.clone()).range); + + Diagnostic::new("macro-error", d.message.clone(), display_range).experimental() } #[cfg(test)] @@ -30,10 +30,10 @@ macro_rules! include { () => {} } macro_rules! compile_error { () => {} } include!("doesntexist"); -//^^^^^^^^^^^^^^^^^^^^^^^^ error: failed to load file `doesntexist` +//^^^^^^^ error: failed to load file `doesntexist` compile_error!("compile_error macro works"); -//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: compile_error macro works +//^^^^^^^^^^^^^ error: compile_error macro works "#, ); } @@ -111,7 +111,7 @@ macro_rules! env { () => {} } macro_rules! concat { () => {} } include!(concat!(env!("OUT_DIR"), "/out.rs")); -//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `OUT_DIR` not set, enable "build scripts" to fix +//^^^^^^^ error: `OUT_DIR` not set, enable "build scripts" to fix "#, ); } @@ -153,23 +153,23 @@ fn main() { // Test a handful of built-in (eager) macros: include!(invalid); - //^^^^^^^^^^^^^^^^^ error: could not convert tokens + //^^^^^^^ error: could not convert tokens include!("does not exist"); - //^^^^^^^^^^^^^^^^^^^^^^^^^^ error: failed to load file `does not exist` + //^^^^^^^ error: failed to load file `does not exist` env!(invalid); - //^^^^^^^^^^^^^ error: could not convert tokens + //^^^ error: could not convert tokens env!("OUT_DIR"); - //^^^^^^^^^^^^^^^ error: `OUT_DIR` not set, enable "build scripts" to fix + //^^^ error: `OUT_DIR` not set, enable "build scripts" to fix compile_error!("compile_error works"); - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: compile_error works + //^^^^^^^^^^^^^ error: compile_error works // Lazy: format_args!(); - //^^^^^^^^^^^^^^ error: no rule matches input tokens + //^^^^^^^^^^^ error: no rule matches input tokens } "#, ); @@ -186,7 +186,7 @@ fn f() { m!(); m!(hi); - //^^^^^^ error: leftover tokens + //^ error: leftover tokens } "#, ); diff --git a/crates/ide-diagnostics/src/handlers/unresolved_macro_call.rs b/crates/ide-diagnostics/src/handlers/unresolved_macro_call.rs index 831d082bc7e..4b43124757f 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_macro_call.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_macro_call.rs @@ -1,6 +1,3 @@ -use hir::{db::AstDatabase, InFile}; -use syntax::{ast, AstNode, SyntaxNodePtr}; - use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: unresolved-macro-call @@ -11,25 +8,16 @@ pub(crate) fn unresolved_macro_call( ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMacroCall, ) -> Diagnostic { - let last_path_segment = ctx.sema.db.parse_or_expand(d.macro_call.file_id).and_then(|root| { - let node = d.macro_call.value.to_node(&root); - if let Some(macro_call) = ast::MacroCall::cast(node) { - macro_call - .path() - .and_then(|it| it.segment()) - .and_then(|it| it.name_ref()) - .map(|it| InFile::new(d.macro_call.file_id, SyntaxNodePtr::new(it.syntax()))) - } else { - None - } - }); - let diagnostics = last_path_segment.unwrap_or_else(|| d.macro_call.clone().map(|it| it.into())); + // Use more accurate position if available. + let display_range = d + .precise_location + .unwrap_or_else(|| ctx.sema.diagnostics_display_range(d.macro_call.clone()).range); let bang = if d.is_bang { "!" } else { "" }; Diagnostic::new( "unresolved-macro-call", format!("unresolved macro `{}{}`", d.path, bang), - ctx.sema.diagnostics_display_range(diagnostics).range, + display_range, ) .experimental() } diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs index 6f883b9e846..2e8e93c926c 100644 --- a/crates/project-model/src/tests.rs +++ b/crates/project-model/src/tests.rs @@ -172,8 +172,8 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -247,8 +247,8 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -312,8 +312,8 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { }, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: Some( @@ -389,8 +389,8 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -464,8 +464,8 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -554,8 +554,8 @@ fn cargo_hello_world_project_model_with_selective_overrides() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -631,8 +631,8 @@ fn cargo_hello_world_project_model_with_selective_overrides() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -696,8 +696,8 @@ fn cargo_hello_world_project_model_with_selective_overrides() { }, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: Some( @@ -775,8 +775,8 @@ fn cargo_hello_world_project_model_with_selective_overrides() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -852,8 +852,8 @@ fn cargo_hello_world_project_model_with_selective_overrides() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -933,8 +933,8 @@ fn cargo_hello_world_project_model() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -1010,8 +1010,8 @@ fn cargo_hello_world_project_model() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -1075,8 +1075,8 @@ fn cargo_hello_world_project_model() { }, }, dependencies: [], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: Some( @@ -1154,8 +1154,8 @@ fn cargo_hello_world_project_model() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -1231,8 +1231,8 @@ fn cargo_hello_world_project_model() { prelude: true, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no build data", ), origin: CratesIo { repo: None, @@ -1505,8 +1505,8 @@ fn rust_project_hello_world_project_model() { prelude: false, }, ], - proc_macro: Ok( - [], + proc_macro: Err( + "no proc macro dylib present", ), origin: CratesIo { repo: None, diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 6aefb84db8a..18c9c2c8106 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -459,7 +459,7 @@ fn project_json_to_crate_graph( krate.display_name.as_ref().map(|it| it.canonical_name()).unwrap_or(""), &it, ), - None => Ok(Vec::new()), + None => Err("no proc macro dylib present".into()), }; let target_cfgs = match krate.target.as_deref() { @@ -870,9 +870,10 @@ fn add_target_crate_root( } } - let proc_macro = match build_data.as_ref().and_then(|it| it.proc_macro_dylib_path.as_ref()) { - Some(it) => load_proc_macro(it), - None => Ok(Vec::new()), + let proc_macro = match build_data.as_ref().map(|it| it.proc_macro_dylib_path.as_ref()) { + Some(Some(it)) => load_proc_macro(it), + Some(None) => Err("no proc macro dylib present".into()), + None => Err("no build data".into()), }; let display_name = CrateDisplayName::from_canonical_name(cargo_name.to_string()); From 45fd5e697f6d7cf1d09c6c465cb008eabd1c63fb Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 24 Jun 2022 14:19:18 +0200 Subject: [PATCH 2/2] Improve comments --- crates/hir-def/src/nameres/collector.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 09594d1e616..9901df2af08 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -421,15 +421,15 @@ impl DefCollector<'_> { } } - /// When the fixed-point loop reaches a stable state, we might still have some unresolved - /// attributes (or unexpanded attribute proc macros) left over. This takes one of them, and - /// feeds the item it's applied to back into name resolution. + /// When the fixed-point loop reaches a stable state, we might still have + /// some unresolved attributes left over. This takes one of them, and feeds + /// the item it's applied to back into name resolution. /// /// This effectively ignores the fact that the macro is there and just treats the items as /// normal code. /// - /// This improves UX when proc macros are turned off or don't work, and replicates the behavior - /// before we supported proc. attribute macros. + /// This improves UX for unresolved attributes, and replicates the + /// behavior before we supported proc. attribute macros. fn reseed_with_unresolved_attribute(&mut self) -> ReachedFixedPoint { cov_mark::hit!(unresolved_attribute_fallback); @@ -1229,6 +1229,7 @@ impl DefCollector<'_> { ); let loc: MacroCallLoc = self.db.lookup_intern_macro_call(call_id); + // If proc attribute macro expansion is disabled, skip expanding it here if !self.db.enable_proc_attr_macros() { self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( directive.module_id, @@ -1250,8 +1251,10 @@ impl DefCollector<'_> { if let MacroDefKind::ProcMacro(exp, ..) = loc.def.kind { if exp.is_dummy() { - // Proc macros that cannot be expanded are treated as not - // resolved, in order to fall back later. + // If there's no expander for the proc macro (e.g. + // because proc macros are disabled, or building the + // proc macro crate failed), report this and skip + // expansion like we would if it was disabled self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( directive.module_id, loc.kind,