Merge #3561 #3577

3561: feat: add debug code lens r=matklad a=hdevalke

Refs #3539

3577: Protect against infinite macro expansion in def collector r=edwin0cheng a=flodiebold

Something I noticed while trying to make macro expansion more resilient against errors.

There was a test for this, but it wasn't actually working because the first recursive expansion failed. (The comma...)

Even with this limit, that test (when fixed) still takes some time to pass because of the exponential growth of the expansions, so I disabled it and added a different one without growth.

CC @edwin0cheng 

Co-authored-by: Hannes De Valkeneer <[email protected]>
Co-authored-by: hdevalke <[email protected]>
Co-authored-by: Florian Diebold <[email protected]>

Author: 3 people

crates/ra_hir_def/src/nameres/collector.rs

@@ -102,6 +102,7 @@ struct MacroDirective {
     module_id: LocalModuleId,
     ast_id: AstIdWithPath<ast::MacroCall>,
     legacy: Option<MacroCallId>,
+    depth: usize,
 }
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -134,6 +135,7 @@ where
         self.def_map.modules[module_id].origin = ModuleOrigin::CrateRoot { definition: file_id };
         ModCollector {
             def_collector: &mut *self,
+            macro_depth: 0,
             module_id,
             file_id: file_id.into(),
             raw_items: &raw_items,
@@ -516,7 +518,7 @@ where
         macros.retain(|directive| {
             if let Some(call_id) = directive.legacy {
                 res = ReachedFixedPoint::No;
-                resolved.push((directive.module_id, call_id));
+                resolved.push((directive.module_id, call_id, directive.depth));
                 return false;
             }
 
@@ -530,7 +532,7 @@ where
                 );
                 resolved_res.resolved_def.take_macros()
             }) {
-                resolved.push((directive.module_id, call_id));
+                resolved.push((directive.module_id, call_id, directive.depth));
                 res = ReachedFixedPoint::No;
                 return false;
             }
@@ -541,7 +543,7 @@ where
             if let Some(call_id) =
                 directive.ast_id.as_call_id(self.db, |path| self.resolve_attribute_macro(&path))
             {
-                resolved.push((directive.module_id, call_id));
+                resolved.push((directive.module_id, call_id, 0));
                 res = ReachedFixedPoint::No;
                 return false;
             }
@@ -552,8 +554,12 @@ where
         self.unexpanded_macros = macros;
         self.unexpanded_attribute_macros = attribute_macros;
 
-        for (module_id, macro_call_id) in resolved {
-            self.collect_macro_expansion(module_id, macro_call_id);
+        for (module_id, macro_call_id, depth) in resolved {
+            if depth > 1024 {
+                log::debug!("Max macro expansion depth reached");
+                continue;
+            }
+            self.collect_macro_expansion(module_id, macro_call_id, depth);
         }
 
         res
@@ -573,12 +579,18 @@ where
         None
     }
 
-    fn collect_macro_expansion(&mut self, module_id: LocalModuleId, macro_call_id: MacroCallId) {
+    fn collect_macro_expansion(
+        &mut self,
+        module_id: LocalModuleId,
+        macro_call_id: MacroCallId,
+        depth: usize,
+    ) {
         let file_id: HirFileId = macro_call_id.as_file();
         let raw_items = self.db.raw_items(file_id);
         let mod_dir = self.mod_dirs[&module_id].clone();
         ModCollector {
             def_collector: &mut *self,
+            macro_depth: depth,
             file_id,
             module_id,
             raw_items: &raw_items,
@@ -595,6 +607,7 @@ where
 /// Walks a single module, populating defs, imports and macros
 struct ModCollector<'a, D> {
     def_collector: D,
+    macro_depth: usize,
     module_id: LocalModuleId,
     file_id: HirFileId,
     raw_items: &'a raw::RawItems,
@@ -684,6 +697,7 @@ where
 
                 ModCollector {
                     def_collector: &mut *self.def_collector,
+                    macro_depth: self.macro_depth,
                     module_id,
                     file_id: self.file_id,
                     raw_items: self.raw_items,
@@ -713,6 +727,7 @@ where
                         let raw_items = self.def_collector.db.raw_items(file_id.into());
                         ModCollector {
                             def_collector: &mut *self.def_collector,
+                            macro_depth: self.macro_depth,
                             module_id,
                             file_id: file_id.into(),
                             raw_items: &raw_items,
@@ -887,6 +902,7 @@ where
                 module_id: self.module_id,
                 ast_id,
                 legacy: Some(macro_call_id),
+                depth: self.macro_depth + 1,
             });
 
             return;
@@ -902,6 +918,7 @@ where
             module_id: self.module_id,
             ast_id,
             legacy: None,
+            depth: self.macro_depth + 1,
         });
     }
 
@@ -971,13 +988,26 @@ mod tests {
     }
 
     #[test]
-    fn test_macro_expand_will_stop() {
+    fn test_macro_expand_will_stop_1() {
+        do_resolve(
+            r#"
+        macro_rules! foo {
+            ($($ty:ty)*) => { foo!($($ty)*); }
+        }
+        foo!(KABOOM);
+        "#,
+        );
+    }
+
+    #[ignore] // this test does succeed, but takes quite a while :/
+    #[test]
+    fn test_macro_expand_will_stop_2() {
         do_resolve(
             r#"
         macro_rules! foo {
-            ($($ty:ty)*) => { foo!($($ty)*, $($ty)*); }
+            ($($ty:ty)*) => { foo!($($ty)* $($ty)*); }
         }
-foo!(KABOOM);
+        foo!(KABOOM);
         "#,
         );
     }

crates/rust-analyzer/src/cargo_target_spec.rs

@@ -19,50 +19,48 @@ impl CargoTargetSpec {
     pub(crate) fn runnable_args(
         spec: Option<CargoTargetSpec>,
         kind: &RunnableKind,
-    ) -> Result<Vec<String>> {
-        let mut res = Vec::new();
+    ) -> Result<(Vec<String>, Vec<String>)> {
+        let mut args = Vec::new();
+        let mut extra_args = Vec::new();
         match kind {
             RunnableKind::Test { test_id } => {
-                res.push("test".to_string());
+                args.push("test".to_string());
                 if let Some(spec) = spec {
-                    spec.push_to(&mut res);
+                    spec.push_to(&mut args);
                 }
-                res.push("--".to_string());
-                res.push(test_id.to_string());
+                extra_args.push(test_id.to_string());
                 if let TestId::Path(_) = test_id {
-                    res.push("--exact".to_string());
+                    extra_args.push("--exact".to_string());
                 }
-                res.push("--nocapture".to_string());
+                extra_args.push("--nocapture".to_string());
             }
             RunnableKind::TestMod { path } => {
-                res.push("test".to_string());
+                args.push("test".to_string());
                 if let Some(spec) = spec {
-                    spec.push_to(&mut res);
+                    spec.push_to(&mut args);
                 }
-                res.push("--".to_string());
-                res.push(path.to_string());
-                res.push("--nocapture".to_string());
+                extra_args.push(path.to_string());
+                extra_args.push("--nocapture".to_string());
             }
             RunnableKind::Bench { test_id } => {
-                res.push("bench".to_string());
+                args.push("bench".to_string());
                 if let Some(spec) = spec {
-                    spec.push_to(&mut res);
+                    spec.push_to(&mut args);
                 }
-                res.push("--".to_string());
-                res.push(test_id.to_string());
+                extra_args.push(test_id.to_string());
                 if let TestId::Path(_) = test_id {
-                    res.push("--exact".to_string());
+                    extra_args.push("--exact".to_string());
                 }
-                res.push("--nocapture".to_string());
+                extra_args.push("--nocapture".to_string());
             }
             RunnableKind::Bin => {
-                res.push("run".to_string());
+                args.push("run".to_string());
                 if let Some(spec) = spec {
-                    spec.push_to(&mut res);
+                    spec.push_to(&mut args);
                 }
             }
         }
-        Ok(res)
+        Ok((args, extra_args))
     }
 
     pub(crate) fn for_file(

crates/rust-analyzer/src/config.rs

@@ -55,6 +55,9 @@ pub struct ServerConfig {
 
     /// Cargo feature configurations.
     pub cargo_features: CargoFeatures,
+
+    /// Enabled if the vscode_lldb extension is available.
+    pub vscode_lldb: bool,
 }
 
 impl Default for ServerConfig {
@@ -76,6 +79,7 @@ impl Default for ServerConfig {
             additional_out_dirs: FxHashMap::default(),
             cargo_features: Default::default(),
             rustfmt_args: Vec::new(),
+            vscode_lldb: false,
         }
     }
 }

crates/rust-analyzer/src/main_loop.rs

@@ -189,6 +189,7 @@ pub fn main_loop(
                     all_targets: config.cargo_watch_all_targets,
                 },
                 rustfmt_args: config.rustfmt_args,
+                vscode_lldb: config.vscode_lldb,
             }
         };
 

crates/rust-analyzer/src/main_loop/handlers.rs

@@ -381,6 +381,7 @@ pub fn handle_runnables(
         label,
         bin: "cargo".to_string(),
         args: check_args,
+        extra_args: Vec::new(),
         env: FxHashMap::default(),
         cwd: workspace_root.map(|root| root.to_string_lossy().to_string()),
     });
@@ -794,18 +795,35 @@ pub fn handle_code_lens(
             RunnableKind::Bin => "Run",
         }
         .to_string();
-        let r = to_lsp_runnable(&world, file_id, runnable)?;
+        let mut r = to_lsp_runnable(&world, file_id, runnable)?;
         let lens = CodeLens {
             range: r.range,
             command: Some(Command {
                 title,
                 command: "rust-analyzer.runSingle".into(),
-                arguments: Some(vec![to_value(r).unwrap()]),
+                arguments: Some(vec![to_value(&r).unwrap()]),
             }),
             data: None,
         };
-
         lenses.push(lens);
+
+        if world.options.vscode_lldb {
+            if r.args[0] == "run" {
+                r.args[0] = "build".into();
+            } else {
+                r.args.push("--no-run".into());
+            }
+            let debug_lens = CodeLens {
+                range: r.range,
+                command: Some(Command {
+                    title: "Debug".into(),
+                    command: "rust-analyzer.debugSingle".into(),
+                    arguments: Some(vec![to_value(r).unwrap()]),
+                }),
+                data: None,
+            };
+            lenses.push(debug_lens);
+        }
     }
 
     // Handle impls
@@ -952,7 +970,7 @@ fn to_lsp_runnable(
     runnable: Runnable,
 ) -> Result<req::Runnable> {
     let spec = CargoTargetSpec::for_file(world, file_id)?;
-    let args = CargoTargetSpec::runnable_args(spec, &runnable.kind)?;
+    let (args, extra_args) = CargoTargetSpec::runnable_args(spec, &runnable.kind)?;
     let line_index = world.analysis().file_line_index(file_id)?;
     let label = match &runnable.kind {
         RunnableKind::Test { test_id } => format!("test {}", test_id),
@@ -965,6 +983,7 @@ fn to_lsp_runnable(
         label,
         bin: "cargo".to_string(),
         args,
+        extra_args,
         env: {
             let mut m = FxHashMap::default();
             m.insert("RUST_BACKTRACE".to_string(), "short".to_string());
@@ -973,6 +992,7 @@ fn to_lsp_runnable(
         cwd: world.workspace_root_for(file_id).map(|root| root.to_string_lossy().to_string()),
     })
 }
+
 fn highlight(world: &WorldSnapshot, file_id: FileId) -> Result<Vec<Decoration>> {
     let line_index = world.analysis().file_line_index(file_id)?;
     let res = world

crates/rust-analyzer/src/req.rs

@@ -169,6 +169,7 @@ pub struct Runnable {
     pub label: String,
     pub bin: String,
     pub args: Vec<String>,
+    pub extra_args: Vec<String>,
     pub env: FxHashMap<String, String>,
     pub cwd: Option<String>,
 }

crates/rust-analyzer/src/world.rs

@@ -38,6 +38,7 @@ pub struct Options {
     pub inlay_hints: InlayHintsOptions,
     pub rustfmt_args: Vec<String>,
     pub cargo_watch: CheckOptions,
+    pub vscode_lldb: bool,
 }
 
 /// `WorldState` is the primary mutable state of the language server

crates/rust-analyzer/tests/heavy_tests/main.rs

@@ -75,7 +75,8 @@ fn foo() {
         RunnablesParams { text_document: server.doc_id("lib.rs"), position: None },
         json!([
           {
-            "args": [ "test", "--", "foo", "--nocapture" ],
+            "args": [ "test" ],
+            "extraArgs": [ "foo", "--nocapture" ],
             "bin": "cargo",
             "env": { "RUST_BACKTRACE": "short" },
             "cwd": null,
@@ -90,6 +91,7 @@ fn foo() {
               "check",
               "--all"
             ],
+            "extraArgs": [],
             "bin": "cargo",
             "env": {},
             "cwd": null,
@@ -141,13 +143,11 @@ fn main() {}
 
     server.wait_until_workspace_is_loaded();
     server.request::<Runnables>(
-        RunnablesParams {
-            text_document: server.doc_id("foo/tests/spam.rs"),
-            position: None,
-        },
+        RunnablesParams { text_document: server.doc_id("foo/tests/spam.rs"), position: None },
         json!([
           {
-            "args": [ "test", "--package", "foo", "--test", "spam", "--", "test_eggs", "--exact", "--nocapture" ],
+            "args": [ "test", "--package", "foo", "--test", "spam" ],
+            "extraArgs": [ "test_eggs", "--exact", "--nocapture" ],
             "bin": "cargo",
             "env": { "RUST_BACKTRACE": "short" },
             "label": "test test_eggs",
@@ -165,6 +165,7 @@ fn main() {}
               "--test",
               "spam"
             ],
+            "extraArgs": [],
             "bin": "cargo",
             "env": {},
             "cwd": server.path().join("foo"),
@@ -180,7 +181,7 @@ fn main() {}
               }
             }
           }
-        ])
+        ]),
     );
 }
 

editors/code/src/client.ts

@@ -46,6 +46,7 @@ export async function createClient(config: Config, serverPath: string): Promise<
             withSysroot: config.withSysroot,
             cargoFeatures: config.cargoFeatures,
             rustfmtArgs: config.rustfmtArgs,
+            vscodeLldb: vscode.extensions.getExtension("vadimcn.vscode-lldb") != null,
         },
         traceOutputChannel,
         middleware: {