From e728026bc5cbbde6a7890c268d38efa55b192069 Mon Sep 17 00:00:00 2001 From: Ishan Jain Date: Wed, 12 Jun 2024 11:03:04 +0530 Subject: [PATCH 1/8] hir/semantics: Only allow expansion of specific built in macros --- .../rust-analyzer/crates/hir/src/semantics.rs | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/tools/rust-analyzer/crates/hir/src/semantics.rs b/src/tools/rust-analyzer/crates/hir/src/semantics.rs index a38cef2fd5de..51510dd400d8 100644 --- a/src/tools/rust-analyzer/crates/hir/src/semantics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/semantics.rs @@ -19,8 +19,12 @@ use hir_def::{ AsMacroCall, DefWithBodyId, FunctionId, MacroId, TraitId, VariantId, }; use hir_expand::{ - attrs::collect_attrs, db::ExpandDatabase, files::InRealFile, name::AsName, InMacroFile, - MacroCallId, MacroFileId, MacroFileIdExt, + attrs::collect_attrs, + builtin_fn_macro::{BuiltinFnLikeExpander, EagerExpander}, + db::ExpandDatabase, + files::InRealFile, + name::AsName, + InMacroFile, MacroCallId, MacroFileId, MacroFileIdExt, }; use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet}; @@ -319,6 +323,30 @@ impl<'db> SemanticsImpl<'db> { } else { sa.expand(self.db, macro_call)? }; + let macro_call = self.db.lookup_intern_macro_call(file_id.macro_call_id); + + match macro_call.def.kind { + hir_expand::MacroDefKind::BuiltIn( + _, + BuiltinFnLikeExpander::Cfg + | BuiltinFnLikeExpander::StdPanic + | BuiltinFnLikeExpander::Stringify + | BuiltinFnLikeExpander::CorePanic, + ) + | hir_expand::MacroDefKind::BuiltInEager( + _, + EagerExpander::Env + | EagerExpander::Concat + | EagerExpander::Include + | EagerExpander::OptionEnv + | EagerExpander::IncludeStr + | EagerExpander::ConcatBytes + | EagerExpander::IncludeBytes, + ) => { + // Do nothing and allow matching macros to be expanded + } + _ => return None, + } let node = self.parse_or_expand(file_id.into()); Some(node) From 8ab0fc33534b3b57c4f258f5f5221624d22a5b63 Mon Sep 17 00:00:00 2001 From: Ishan Jain Date: Fri, 14 Jun 2024 09:17:56 +0530 Subject: [PATCH 2/8] added tests --- .../crates/ide/src/expand_macro.rs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs b/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs index 1ead045788ff..c411248c4823 100644 --- a/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs +++ b/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs @@ -228,6 +228,32 @@ mod tests { expect.assert_eq(&actual); } + #[test] + fn only_expand_allowed_builtin_macro() { + let fail_tests = [r#" + //- minicore: asm + $0asm!("0x300, x0"); + "#]; + + for test in fail_tests { + let (analysis, pos) = fixture::position(test); + let expansion = analysis.expand_macro(pos).unwrap(); + assert!(expansion.is_none()); + } + + let tests = [( + r#" + //- minicore: concat + $0concat!("test", 10, 'b', true);"#, + expect![[r#" + concat! + "test10btrue""#]], + )]; + for (test, expect) in tests { + check(test, expect); + } + } + #[test] fn macro_expand_as_keyword() { check( From 4d58fc1729d046071976bb58c13621e2779c7166 Mon Sep 17 00:00:00 2001 From: Ishan Jain Date: Fri, 14 Jun 2024 09:52:46 +0530 Subject: [PATCH 3/8] fixed tests --- src/tools/rust-analyzer/crates/hir/src/semantics.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tools/rust-analyzer/crates/hir/src/semantics.rs b/src/tools/rust-analyzer/crates/hir/src/semantics.rs index 51510dd400d8..d17a33cc2e76 100644 --- a/src/tools/rust-analyzer/crates/hir/src/semantics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/semantics.rs @@ -345,7 +345,13 @@ impl<'db> SemanticsImpl<'db> { ) => { // Do nothing and allow matching macros to be expanded } - _ => return None, + + hir_expand::MacroDefKind::BuiltIn(_, _) + | hir_expand::MacroDefKind::BuiltInAttr(_, _) + | hir_expand::MacroDefKind::BuiltInEager(_, _) + | hir_expand::MacroDefKind::BuiltInDerive(_, _) => return None, + + _ => (), } let node = self.parse_or_expand(file_id.into()); From 1623f151669ad3894e8f6c7e700d821f17ddc16e Mon Sep 17 00:00:00 2001 From: Ishan Jain Date: Fri, 14 Jun 2024 10:13:41 +0530 Subject: [PATCH 4/8] allow format_args! expansion --- src/tools/rust-analyzer/crates/hir/src/semantics.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/rust-analyzer/crates/hir/src/semantics.rs b/src/tools/rust-analyzer/crates/hir/src/semantics.rs index d17a33cc2e76..d11731ff2a5a 100644 --- a/src/tools/rust-analyzer/crates/hir/src/semantics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/semantics.rs @@ -331,7 +331,8 @@ impl<'db> SemanticsImpl<'db> { BuiltinFnLikeExpander::Cfg | BuiltinFnLikeExpander::StdPanic | BuiltinFnLikeExpander::Stringify - | BuiltinFnLikeExpander::CorePanic, + | BuiltinFnLikeExpander::CorePanic + | BuiltinFnLikeExpander::FormatArgs, ) | hir_expand::MacroDefKind::BuiltInEager( _, From 9b619035b9d1e13dc24679a5424af484a598131a Mon Sep 17 00:00:00 2001 From: Ishan Jain Date: Sat, 15 Jun 2024 15:20:46 +0530 Subject: [PATCH 5/8] Created expand_allowed_builtins, updated expand_macro to call this function --- .../rust-analyzer/crates/hir/src/semantics.rs | 16 ++++++++++++++++ .../rust-analyzer/crates/ide/src/expand_macro.rs | 7 ++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/tools/rust-analyzer/crates/hir/src/semantics.rs b/src/tools/rust-analyzer/crates/hir/src/semantics.rs index d11731ff2a5a..8261828e0269 100644 --- a/src/tools/rust-analyzer/crates/hir/src/semantics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/semantics.rs @@ -315,6 +315,22 @@ impl<'db> SemanticsImpl<'db> { pub fn expand(&self, macro_call: &ast::MacroCall) -> Option { let sa = self.analyze_no_infer(macro_call.syntax())?; + let macro_call = InFile::new(sa.file_id, macro_call); + let file_id = if let Some(call) = + ::to_def(self, macro_call) + { + call.as_macro_file() + } else { + sa.expand(self.db, macro_call)? + }; + + let node = self.parse_or_expand(file_id.into()); + Some(node) + } + + pub fn expand_allowed_builtins(&self, macro_call: &ast::MacroCall) -> Option { + let sa = self.analyze_no_infer(macro_call.syntax())?; + let macro_call = InFile::new(sa.file_id, macro_call); let file_id = if let Some(call) = ::to_def(self, macro_call) diff --git a/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs b/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs index c411248c4823..55838799f6e8 100644 --- a/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs +++ b/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs @@ -111,9 +111,10 @@ fn expand_macro_recur( macro_call: &ast::Item, ) -> Option { let expanded = match macro_call { - item @ ast::Item::MacroCall(macro_call) => { - sema.expand_attr_macro(item).or_else(|| sema.expand(macro_call))?.clone_for_update() - } + item @ ast::Item::MacroCall(macro_call) => sema + .expand_attr_macro(item) + .or_else(|| sema.expand_allowed_builtins(macro_call))? + .clone_for_update(), item => sema.expand_attr_macro(item)?.clone_for_update(), }; expand(sema, expanded) From 33d4ab65e81e7be0357b76833841ae2a054e95e8 Mon Sep 17 00:00:00 2001 From: Ishan Jain Date: Wed, 19 Jun 2024 16:11:50 +0530 Subject: [PATCH 6/8] updated tests --- .../crates/ide/src/expand_macro.rs | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs b/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs index 55838799f6e8..e3bb159daf43 100644 --- a/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs +++ b/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs @@ -230,29 +230,26 @@ mod tests { } #[test] - fn only_expand_allowed_builtin_macro() { - let fail_tests = [r#" - //- minicore: asm - $0asm!("0x300, x0"); - "#]; - - for test in fail_tests { - let (analysis, pos) = fixture::position(test); - let expansion = analysis.expand_macro(pos).unwrap(); - assert!(expansion.is_none()); - } - - let tests = [( + fn expand_allowed_builtin_macro() { + check( r#" //- minicore: concat $0concat!("test", 10, 'b', true);"#, expect![[r#" concat! "test10btrue""#]], - )]; - for (test, expect) in tests { - check(test, expect); - } + ); + } + + #[test] + fn do_not_expand_disallowed_macro() { + let (analysis, pos) = fixture::position( + r#" + //- minicore: asm + $0asm!("0x300, x0");"#, + ); + let expansion = analysis.expand_macro(pos).unwrap(); + assert!(expansion.is_none()); } #[test] From a2d4e2934e86c45cf3c343650ab968c7edd05e45 Mon Sep 17 00:00:00 2001 From: Ishan Jain Date: Wed, 19 Jun 2024 16:58:48 +0530 Subject: [PATCH 7/8] removed format_args from allowed expansions --- src/tools/rust-analyzer/crates/hir/src/semantics.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools/rust-analyzer/crates/hir/src/semantics.rs b/src/tools/rust-analyzer/crates/hir/src/semantics.rs index 8261828e0269..633d2aaf3e8f 100644 --- a/src/tools/rust-analyzer/crates/hir/src/semantics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/semantics.rs @@ -347,8 +347,7 @@ impl<'db> SemanticsImpl<'db> { BuiltinFnLikeExpander::Cfg | BuiltinFnLikeExpander::StdPanic | BuiltinFnLikeExpander::Stringify - | BuiltinFnLikeExpander::CorePanic - | BuiltinFnLikeExpander::FormatArgs, + | BuiltinFnLikeExpander::CorePanic, ) | hir_expand::MacroDefKind::BuiltInEager( _, From e5d5c7b20ad3e74b85fe9c3af2852595ae8a345a Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 20 Jun 2024 10:31:20 +0200 Subject: [PATCH 8/8] Invert matching on builtin macros in expand_allowed_builtins --- .../rust-analyzer/crates/hir/src/semantics.rs | 46 +++++++++---------- .../crates/ide/src/expand_macro.rs | 8 ++-- 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/src/tools/rust-analyzer/crates/hir/src/semantics.rs b/src/tools/rust-analyzer/crates/hir/src/semantics.rs index 633d2aaf3e8f..358f10d3b897 100644 --- a/src/tools/rust-analyzer/crates/hir/src/semantics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/semantics.rs @@ -328,6 +328,8 @@ impl<'db> SemanticsImpl<'db> { Some(node) } + /// Expands the macro if it isn't one of the built-in ones that expand to custom syntax or dummy + /// expansions. pub fn expand_allowed_builtins(&self, macro_call: &ast::MacroCall) -> Option { let sa = self.analyze_no_infer(macro_call.syntax())?; @@ -341,33 +343,27 @@ impl<'db> SemanticsImpl<'db> { }; let macro_call = self.db.lookup_intern_macro_call(file_id.macro_call_id); - match macro_call.def.kind { + let skip = matches!( + macro_call.def.kind, hir_expand::MacroDefKind::BuiltIn( _, - BuiltinFnLikeExpander::Cfg - | BuiltinFnLikeExpander::StdPanic - | BuiltinFnLikeExpander::Stringify - | BuiltinFnLikeExpander::CorePanic, - ) - | hir_expand::MacroDefKind::BuiltInEager( - _, - EagerExpander::Env - | EagerExpander::Concat - | EagerExpander::Include - | EagerExpander::OptionEnv - | EagerExpander::IncludeStr - | EagerExpander::ConcatBytes - | EagerExpander::IncludeBytes, - ) => { - // Do nothing and allow matching macros to be expanded - } - - hir_expand::MacroDefKind::BuiltIn(_, _) - | hir_expand::MacroDefKind::BuiltInAttr(_, _) - | hir_expand::MacroDefKind::BuiltInEager(_, _) - | hir_expand::MacroDefKind::BuiltInDerive(_, _) => return None, - - _ => (), + BuiltinFnLikeExpander::Column + | BuiltinFnLikeExpander::File + | BuiltinFnLikeExpander::ModulePath + | BuiltinFnLikeExpander::Asm + | BuiltinFnLikeExpander::LlvmAsm + | BuiltinFnLikeExpander::GlobalAsm + | BuiltinFnLikeExpander::LogSyntax + | BuiltinFnLikeExpander::TraceMacros + | BuiltinFnLikeExpander::FormatArgs + | BuiltinFnLikeExpander::FormatArgsNl + | BuiltinFnLikeExpander::ConstFormatArgs, + ) | hir_expand::MacroDefKind::BuiltInEager(_, EagerExpander::CompileError) + ); + if skip { + // these macros expand to custom builtin syntax and/or dummy things, no point in + // showing these to the user + return None; } let node = self.parse_or_expand(file_id.into()); diff --git a/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs b/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs index e3bb159daf43..4b54c057bf33 100644 --- a/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs +++ b/src/tools/rust-analyzer/crates/ide/src/expand_macro.rs @@ -233,8 +233,8 @@ mod tests { fn expand_allowed_builtin_macro() { check( r#" - //- minicore: concat - $0concat!("test", 10, 'b', true);"#, +//- minicore: concat +$0concat!("test", 10, 'b', true);"#, expect![[r#" concat! "test10btrue""#]], @@ -245,8 +245,8 @@ mod tests { fn do_not_expand_disallowed_macro() { let (analysis, pos) = fixture::position( r#" - //- minicore: asm - $0asm!("0x300, x0");"#, +//- minicore: asm +$0asm!("0x300, x0");"#, ); let expansion = analysis.expand_macro(pos).unwrap(); assert!(expansion.is_none());