aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-03-17 07:20:28 +0000
committerGitHub <[email protected]>2021-03-17 07:20:28 +0000
commit6fcb5d772f16af0d1f62dad55fbde75072fb9e89 (patch)
treee4ddd896f9ca8ca1f99c346a49104b04bc4ca04c
parent83e6940efb42675226adb8d2856c095b8dce36c5 (diff)
parenta79b5673e8f5d1f8d569bc7c984a293a972a7bb0 (diff)
Merge #8048
8048: Fix missing unresolved macro diagnostic in function body r=edwin0cheng a=brandondong This was an issue I found while working on https://github.com/rust-analyzer/rust-analyzer/pull/7970. **Reproduction:** 1. Call a non-existent macro in a function body. ``` fn main() { foo!(); } ``` 2. No diagnostics are raised. An unresolved-macro-call diagnostic is expected. 3. If the macro call is instead outside of the function body, this works as expected. I believe this worked previously and regressed in https://github.com/rust-analyzer/rust-analyzer/pull/7805. **Behavior prior to https://github.com/rust-analyzer/rust-analyzer/pull/7805:** - The unresolved-macro-call diagnostic did not exist. Instead, a macro-error diagnostic would be raised with the text "could not resolve macro [path]". - This was implemented by adding an error to the error sink (https://github.com/rust-analyzer/rust-analyzer/pull/7805/files#diff-50a326c5ae465bd9b31ee4310186380aa06e4fa1f6b41dbc0aed5bcc656a3cb8L657). - The error was propagated through https://github.com/rust-analyzer/rust-analyzer/blob/1a82af3527e476d52410ff4dfd2fb4c57466abcb/crates/hir_def/src/body.rs#L123 eventually reaching https://github.com/rust-analyzer/rust-analyzer/blob/1a82af3527e476d52410ff4dfd2fb4c57466abcb/crates/hir_def/src/body/lower.rs#L569. **Behavior after:** - Instead of writing to the error sink, an UnresolvedMacro error is now returned (https://github.com/rust-analyzer/rust-analyzer/pull/7805/files#diff-50a326c5ae465bd9b31ee4310186380aa06e4fa1f6b41dbc0aed5bcc656a3cb8R631). - The parent caller throws away the error as its function signature is `Option<MacroCallId>` (https://github.com/rust-analyzer/rust-analyzer/pull/7805/files#diff-50a326c5ae465bd9b31ee4310186380aa06e4fa1f6b41dbc0aed5bcc656a3cb8R604). - We instead now reach the warn condition (https://github.com/rust-analyzer/rust-analyzer/blob/1a82af3527e476d52410ff4dfd2fb4c57466abcb/crates/hir_def/src/body.rs#L124) and no diagnostics are created in https://github.com/rust-analyzer/rust-analyzer/blob/1a82af3527e476d52410ff4dfd2fb4c57466abcb/crates/hir_def/src/body/lower.rs#L575. **Fix:** - Make sure to propagate the UnresolvedMacro error. Report the error using the new unresolved-macro-call diagnostic. Co-authored-by: Brandon <[email protected]>
-rw-r--r--crates/hir_def/src/body.rs26
-rw-r--r--crates/hir_def/src/body/diagnostics.rs6
-rw-r--r--crates/hir_def/src/body/lower.rs15
-rw-r--r--crates/hir_def/src/body/tests.rs12
-rw-r--r--crates/hir_def/src/data.rs37
-rw-r--r--crates/hir_def/src/diagnostics.rs2
-rw-r--r--crates/hir_def/src/lib.rs27
-rw-r--r--crates/hir_expand/src/eager.rs2
8 files changed, 80 insertions, 47 deletions
diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs
index 8bcc350ce..1080d9c2c 100644
--- a/crates/hir_def/src/body.rs
+++ b/crates/hir_def/src/body.rs
@@ -32,6 +32,7 @@ use crate::{
32 path::{ModPath, Path}, 32 path::{ModPath, Path},
33 src::HasSource, 33 src::HasSource,
34 AsMacroCall, BlockId, DefWithBodyId, HasModule, LocalModuleId, Lookup, ModuleId, 34 AsMacroCall, BlockId, DefWithBodyId, HasModule, LocalModuleId, Lookup, ModuleId,
35 UnresolvedMacro,
35}; 36};
36 37
37/// A subset of Expander that only deals with cfg attributes. We only need it to 38/// A subset of Expander that only deals with cfg attributes. We only need it to
@@ -101,10 +102,12 @@ impl Expander {
101 &mut self, 102 &mut self,
102 db: &dyn DefDatabase, 103 db: &dyn DefDatabase,
103 macro_call: ast::MacroCall, 104 macro_call: ast::MacroCall,
104 ) -> ExpandResult<Option<(Mark, T)>> { 105 ) -> Result<ExpandResult<Option<(Mark, T)>>, UnresolvedMacro> {
105 if self.recursion_limit + 1 > EXPANSION_RECURSION_LIMIT { 106 if self.recursion_limit + 1 > EXPANSION_RECURSION_LIMIT {
106 cov_mark::hit!(your_stack_belongs_to_me); 107 cov_mark::hit!(your_stack_belongs_to_me);
107 return ExpandResult::str_err("reached recursion limit during macro expansion".into()); 108 return Ok(ExpandResult::str_err(
109 "reached recursion limit during macro expansion".into(),
110 ));
108 } 111 }
109 112
110 let macro_call = InFile::new(self.current_file_id, &macro_call); 113 let macro_call = InFile::new(self.current_file_id, &macro_call);
@@ -116,14 +119,11 @@ impl Expander {
116 let call_id = 119 let call_id =
117 macro_call.as_call_id_with_errors(db, self.def_map.krate(), resolver, &mut |e| { 120 macro_call.as_call_id_with_errors(db, self.def_map.krate(), resolver, &mut |e| {
118 err.get_or_insert(e); 121 err.get_or_insert(e);
119 }); 122 })?;
120 let call_id = match call_id { 123 let call_id = match call_id {
121 Some(it) => it, 124 Ok(it) => it,
122 None => { 125 Err(_) => {
123 if err.is_none() { 126 return Ok(ExpandResult { value: None, err });
124 log::warn!("no error despite `as_call_id_with_errors` returning `None`");
125 }
126 return ExpandResult { value: None, err };
127 } 127 }
128 }; 128 };
129 129
@@ -141,9 +141,9 @@ impl Expander {
141 log::warn!("no error despite `parse_or_expand` failing"); 141 log::warn!("no error despite `parse_or_expand` failing");
142 } 142 }
143 143
144 return ExpandResult::only_err(err.unwrap_or_else(|| { 144 return Ok(ExpandResult::only_err(err.unwrap_or_else(|| {
145 mbe::ExpandError::Other("failed to parse macro invocation".into()) 145 mbe::ExpandError::Other("failed to parse macro invocation".into())
146 })); 146 })));
147 } 147 }
148 }; 148 };
149 149
@@ -151,7 +151,7 @@ impl Expander {
151 Some(it) => it, 151 Some(it) => it,
152 None => { 152 None => {
153 // This can happen without being an error, so only forward previous errors. 153 // This can happen without being an error, so only forward previous errors.
154 return ExpandResult { value: None, err }; 154 return Ok(ExpandResult { value: None, err });
155 } 155 }
156 }; 156 };
157 157
@@ -167,7 +167,7 @@ impl Expander {
167 self.current_file_id = file_id; 167 self.current_file_id = file_id;
168 self.ast_id_map = db.ast_id_map(file_id); 168 self.ast_id_map = db.ast_id_map(file_id);
169 169
170 ExpandResult { value: Some((mark, node)), err } 170 Ok(ExpandResult { value: Some((mark, node)), err })
171 } 171 }
172 172
173 pub(crate) fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) { 173 pub(crate) fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {
diff --git a/crates/hir_def/src/body/diagnostics.rs b/crates/hir_def/src/body/diagnostics.rs
index 1de7d30e2..f6992c9a8 100644
--- a/crates/hir_def/src/body/diagnostics.rs
+++ b/crates/hir_def/src/body/diagnostics.rs
@@ -2,13 +2,14 @@
2 2
3use hir_expand::diagnostics::DiagnosticSink; 3use hir_expand::diagnostics::DiagnosticSink;
4 4
5use crate::diagnostics::{InactiveCode, MacroError, UnresolvedProcMacro}; 5use crate::diagnostics::{InactiveCode, MacroError, UnresolvedMacroCall, UnresolvedProcMacro};
6 6
7#[derive(Debug, Eq, PartialEq)] 7#[derive(Debug, Eq, PartialEq)]
8pub(crate) enum BodyDiagnostic { 8pub(crate) enum BodyDiagnostic {
9 InactiveCode(InactiveCode), 9 InactiveCode(InactiveCode),
10 MacroError(MacroError), 10 MacroError(MacroError),
11 UnresolvedProcMacro(UnresolvedProcMacro), 11 UnresolvedProcMacro(UnresolvedProcMacro),
12 UnresolvedMacroCall(UnresolvedMacroCall),
12} 13}
13 14
14impl BodyDiagnostic { 15impl BodyDiagnostic {
@@ -23,6 +24,9 @@ impl BodyDiagnostic {
23 BodyDiagnostic::UnresolvedProcMacro(diag) => { 24 BodyDiagnostic::UnresolvedProcMacro(diag) => {
24 sink.push(diag.clone()); 25 sink.push(diag.clone());
25 } 26 }
27 BodyDiagnostic::UnresolvedMacroCall(diag) => {
28 sink.push(diag.clone());
29 }
26 } 30 }
27 } 31 }
28} 32}
diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs
index 7052058f2..60b25db56 100644
--- a/crates/hir_def/src/body/lower.rs
+++ b/crates/hir_def/src/body/lower.rs
@@ -24,7 +24,7 @@ use crate::{
24 body::{Body, BodySourceMap, Expander, LabelSource, PatPtr, SyntheticSyntax}, 24 body::{Body, BodySourceMap, Expander, LabelSource, PatPtr, SyntheticSyntax},
25 builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint}, 25 builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint},
26 db::DefDatabase, 26 db::DefDatabase,
27 diagnostics::{InactiveCode, MacroError, UnresolvedProcMacro}, 27 diagnostics::{InactiveCode, MacroError, UnresolvedMacroCall, UnresolvedProcMacro},
28 expr::{ 28 expr::{
29 dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Label, 29 dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Label,
30 LabelId, Literal, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, 30 LabelId, Literal, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField,
@@ -33,7 +33,7 @@ use crate::{
33 item_scope::BuiltinShadowMode, 33 item_scope::BuiltinShadowMode,
34 path::{GenericArgs, Path}, 34 path::{GenericArgs, Path},
35 type_ref::{Mutability, Rawness, TypeRef}, 35 type_ref::{Mutability, Rawness, TypeRef},
36 AdtId, BlockLoc, ModuleDefId, 36 AdtId, BlockLoc, ModuleDefId, UnresolvedMacro,
37}; 37};
38 38
39use super::{diagnostics::BodyDiagnostic, ExprSource, PatSource}; 39use super::{diagnostics::BodyDiagnostic, ExprSource, PatSource};
@@ -554,6 +554,17 @@ impl ExprCollector<'_> {
554 let macro_call = self.expander.to_source(AstPtr::new(&e)); 554 let macro_call = self.expander.to_source(AstPtr::new(&e));
555 let res = self.expander.enter_expand(self.db, e); 555 let res = self.expander.enter_expand(self.db, e);
556 556
557 let res = match res {
558 Ok(res) => res,
559 Err(UnresolvedMacro) => {
560 self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedMacroCall(
561 UnresolvedMacroCall { file: outer_file, node: syntax_ptr.cast().unwrap() },
562 ));
563 collector(self, None);
564 return;
565 }
566 };
567
557 match &res.err { 568 match &res.err {
558 Some(ExpandError::UnresolvedProcMacro) => { 569 Some(ExpandError::UnresolvedProcMacro) => {
559 self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro( 570 self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro(
diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs
index 991a32b15..f8e6f70e8 100644
--- a/crates/hir_def/src/body/tests.rs
+++ b/crates/hir_def/src/body/tests.rs
@@ -175,6 +175,18 @@ fn f() {
175} 175}
176 176
177#[test] 177#[test]
178fn unresolved_macro_diag() {
179 check_diagnostics(
180 r#"
181fn f() {
182 m!();
183 //^^^^ unresolved macro call
184}
185 "#,
186 );
187}
188
189#[test]
178fn dollar_crate_in_builtin_macro() { 190fn dollar_crate_in_builtin_macro() {
179 check_diagnostics( 191 check_diagnostics(
180 r#" 192 r#"
diff --git a/crates/hir_def/src/data.rs b/crates/hir_def/src/data.rs
index 74a2194e5..1a27f7bf2 100644
--- a/crates/hir_def/src/data.rs
+++ b/crates/hir_def/src/data.rs
@@ -267,23 +267,26 @@ fn collect_items(
267 let ast_id_map = db.ast_id_map(file_id); 267 let ast_id_map = db.ast_id_map(file_id);
268 let root = db.parse_or_expand(file_id).unwrap(); 268 let root = db.parse_or_expand(file_id).unwrap();
269 let call = ast_id_map.get(call.ast_id).to_node(&root); 269 let call = ast_id_map.get(call.ast_id).to_node(&root);
270 270 let res = expander.enter_expand(db, call);
271 if let Some((mark, mac)) = expander.enter_expand(db, call).value { 271
272 let src: InFile<ast::MacroItems> = expander.to_source(mac); 272 if let Ok(res) = res {
273 let item_tree = db.item_tree(src.file_id); 273 if let Some((mark, mac)) = res.value {
274 let iter = 274 let src: InFile<ast::MacroItems> = expander.to_source(mac);
275 item_tree.top_level_items().iter().filter_map(ModItem::as_assoc_item); 275 let item_tree = db.item_tree(src.file_id);
276 items.extend(collect_items( 276 let iter =
277 db, 277 item_tree.top_level_items().iter().filter_map(ModItem::as_assoc_item);
278 module, 278 items.extend(collect_items(
279 expander, 279 db,
280 iter, 280 module,
281 src.file_id, 281 expander,
282 container, 282 iter,
283 limit - 1, 283 src.file_id,
284 )); 284 container,
285 285 limit - 1,
286 expander.exit(db, mark); 286 ));
287
288 expander.exit(db, mark);
289 }
287 } 290 }
288 } 291 }
289 } 292 }
diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs
index 7188bb299..97abf8653 100644
--- a/crates/hir_def/src/diagnostics.rs
+++ b/crates/hir_def/src/diagnostics.rs
@@ -99,7 +99,7 @@ impl Diagnostic for UnresolvedImport {
99// 99//
100// This diagnostic is triggered if rust-analyzer is unable to resolve the path to a 100// This diagnostic is triggered if rust-analyzer is unable to resolve the path to a
101// macro in a macro invocation. 101// macro in a macro invocation.
102#[derive(Debug)] 102#[derive(Debug, Clone, Eq, PartialEq)]
103pub struct UnresolvedMacroCall { 103pub struct UnresolvedMacroCall {
104 pub file: HirFileId, 104 pub file: HirFileId,
105 pub node: AstPtr<ast::MacroCall>, 105 pub node: AstPtr<ast::MacroCall>,
diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs
index c6655c5fb..6758411a0 100644
--- a/crates/hir_def/src/lib.rs
+++ b/crates/hir_def/src/lib.rs
@@ -58,7 +58,7 @@ use std::{
58use base_db::{impl_intern_key, salsa, CrateId}; 58use base_db::{impl_intern_key, salsa, CrateId};
59use hir_expand::{ 59use hir_expand::{
60 ast_id_map::FileAstId, 60 ast_id_map::FileAstId,
61 eager::{expand_eager_macro, ErrorEmitted}, 61 eager::{expand_eager_macro, ErrorEmitted, ErrorSink},
62 hygiene::Hygiene, 62 hygiene::Hygiene,
63 AstId, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind, 63 AstId, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
64}; 64};
@@ -583,7 +583,7 @@ pub trait AsMacroCall {
583 krate: CrateId, 583 krate: CrateId,
584 resolver: impl Fn(path::ModPath) -> Option<MacroDefId>, 584 resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
585 ) -> Option<MacroCallId> { 585 ) -> Option<MacroCallId> {
586 self.as_call_id_with_errors(db, krate, resolver, &mut |_| ()) 586 self.as_call_id_with_errors(db, krate, resolver, &mut |_| ()).ok()?.ok()
587 } 587 }
588 588
589 fn as_call_id_with_errors( 589 fn as_call_id_with_errors(
@@ -592,7 +592,7 @@ pub trait AsMacroCall {
592 krate: CrateId, 592 krate: CrateId,
593 resolver: impl Fn(path::ModPath) -> Option<MacroDefId>, 593 resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
594 error_sink: &mut dyn FnMut(mbe::ExpandError), 594 error_sink: &mut dyn FnMut(mbe::ExpandError),
595 ) -> Option<MacroCallId>; 595 ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro>;
596} 596}
597 597
598impl AsMacroCall for InFile<&ast::MacroCall> { 598impl AsMacroCall for InFile<&ast::MacroCall> {
@@ -601,25 +601,28 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
601 db: &dyn db::DefDatabase, 601 db: &dyn db::DefDatabase,
602 krate: CrateId, 602 krate: CrateId,
603 resolver: impl Fn(path::ModPath) -> Option<MacroDefId>, 603 resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
604 error_sink: &mut dyn FnMut(mbe::ExpandError), 604 mut error_sink: &mut dyn FnMut(mbe::ExpandError),
605 ) -> Option<MacroCallId> { 605 ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
606 let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value)); 606 let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));
607 let h = Hygiene::new(db.upcast(), self.file_id); 607 let h = Hygiene::new(db.upcast(), self.file_id);
608 let path = self.value.path().and_then(|path| path::ModPath::from_src(path, &h)); 608 let path = self.value.path().and_then(|path| path::ModPath::from_src(path, &h));
609 609
610 if path.is_none() { 610 let path = match error_sink
611 error_sink(mbe::ExpandError::Other("malformed macro invocation".into())); 611 .option(path, || mbe::ExpandError::Other("malformed macro invocation".into()))
612 } 612 {
613 Ok(path) => path,
614 Err(error) => {
615 return Ok(Err(error));
616 }
617 };
613 618
614 macro_call_as_call_id( 619 macro_call_as_call_id(
615 &AstIdWithPath::new(ast_id.file_id, ast_id.value, path?), 620 &AstIdWithPath::new(ast_id.file_id, ast_id.value, path),
616 db, 621 db,
617 krate, 622 krate,
618 resolver, 623 resolver,
619 error_sink, 624 error_sink,
620 ) 625 )
621 .ok()?
622 .ok()
623 } 626 }
624} 627}
625 628
@@ -636,7 +639,7 @@ impl<T: ast::AstNode> AstIdWithPath<T> {
636 } 639 }
637} 640}
638 641
639struct UnresolvedMacro; 642pub struct UnresolvedMacro;
640 643
641fn macro_call_as_call_id( 644fn macro_call_as_call_id(
642 call: &AstIdWithPath<ast::MacroCall>, 645 call: &AstIdWithPath<ast::MacroCall>,
diff --git a/crates/hir_expand/src/eager.rs b/crates/hir_expand/src/eager.rs
index ae7b51a08..dc618a9ee 100644
--- a/crates/hir_expand/src/eager.rs
+++ b/crates/hir_expand/src/eager.rs
@@ -35,7 +35,7 @@ pub struct ErrorEmitted {
35 _private: (), 35 _private: (),
36} 36}
37 37
38trait ErrorSink { 38pub trait ErrorSink {
39 fn emit(&mut self, err: mbe::ExpandError); 39 fn emit(&mut self, err: mbe::ExpandError);
40 40
41 fn option<T>( 41 fn option<T>(