Fix dependency tracking for debugger visualizers

michaelwoerister · michaelwoerister · commit 7f01893900cf · 2023-05-16T18:50:26.000+02:00
diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs
@@ -486,6 +486,11 @@ fn write_out_deps(tcx: TyCtxt<'_>, outputs: &OutputFilenames, out_filenames: &[P
             files.push(normalize_path(profile_sample.as_path().to_path_buf()));
         }
 
+        // Debugger visualizer files
+        for debugger_visualizer in tcx.debugger_visualizers(LOCAL_CRATE) {
+            files.push(normalize_path(debugger_visualizer.path.clone().unwrap()));
+        }
+
         if sess.binary_dep_depinfo() {
             if let Some(ref backend) = sess.opts.unstable_opts.codegen_backend {
                 if backend.contains('.') {
@@ -567,6 +572,12 @@ fn resolver_for_lowering<'tcx>(
     // Make sure we don't mutate the cstore from here on.
     tcx.untracked().cstore.leak();
 
+    {
+        let debugger_visualizers = rustc_passes::debugger_visualizer::collect(tcx.sess, &krate);
+        let feed = tcx.feed_local_crate();
+        feed.debugger_visualizers(debugger_visualizers);
+    }
+
     let ty::ResolverOutputs {
         global_ctxt: untracked_resolutions,
         ast_lowering: untracked_resolver_for_lowering,
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -1852,7 +1852,16 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
     fn encode_debugger_visualizers(&mut self) -> LazyArray<DebuggerVisualizerFile> {
         empty_proc_macro!(self);
-        self.lazy_array(self.tcx.debugger_visualizers(LOCAL_CRATE).iter())
+        self.lazy_array(
+            self.tcx
+                .debugger_visualizers(LOCAL_CRATE)
+                .iter()
+                // Erase the path since it may contain privacy sensitive data
+                // that we don't want to end up in crate metadata.
+                // The path is only needed for the local crate because of
+                // `--emit dep-info`.
+                .map(DebuggerVisualizerFile::path_erased),
+        )
     }
 
     fn encode_crate_deps(&mut self) -> LazyArray<CrateDep> {
diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs
@@ -15,7 +15,7 @@ use rustc_index::Idx;
 use rustc_middle::hir::nested_filter;
 use rustc_span::def_id::StableCrateId;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
-use rustc_span::Span;
+use rustc_span::{DebuggerVisualizerFile, Span};
 use rustc_target::spec::abi::Abi;
 
 #[inline]
@@ -1165,11 +1165,21 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh {
 
     source_file_names.sort_unstable();
 
+    let debugger_visualizers: Vec<_> = tcx
+        .debugger_visualizers(LOCAL_CRATE)
+        .iter()
+        // We ignore the path to the visualizer file since it's not going to be
+        // encoded in crate metadata and we already hash the full contents of
+        // the file.
+        .map(DebuggerVisualizerFile::path_erased)
+        .collect();
+
     let crate_hash: Fingerprint = tcx.with_stable_hashing_context(|mut hcx| {
         let mut stable_hasher = StableHasher::new();
         hir_body_hash.hash_stable(&mut hcx, &mut stable_hasher);
         upstream_crates.hash_stable(&mut hcx, &mut stable_hasher);
         source_file_names.hash_stable(&mut hcx, &mut stable_hasher);
+        debugger_visualizers.hash_stable(&mut hcx, &mut stable_hasher);
         if tcx.sess.opts.incremental_relative_spans() {
             let definitions = tcx.definitions_untracked();
             let mut owner_spans: Vec<_> = krate
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
@@ -1795,6 +1795,7 @@ rustc_queries! {
         arena_cache
         desc { "looking up the debugger visualizers for this crate" }
         separate_provide_extern
+        feedable
     }
     query postorder_cnums(_: ()) -> &'tcx [CrateNum] {
         eval_always
diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
@@ -8,7 +8,6 @@ use crate::{errors, fluent_generated as fluent};
 use rustc_ast::{ast, AttrStyle, Attribute, LitKind, MetaItemKind, MetaItemLit, NestedMetaItem};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{Applicability, IntoDiagnosticArg, MultiSpan};
-use rustc_expand::base::resolve_path;
 use rustc_feature::{AttributeDuplicates, AttributeType, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
 use rustc_hir as hir;
 use rustc_hir::def_id::LocalDefId;
@@ -1916,6 +1915,7 @@ impl CheckAttrVisitor<'_> {
 
     /// Checks if the items on the `#[debugger_visualizer]` attribute are valid.
     fn check_debugger_visualizer(&self, attr: &Attribute, target: Target) -> bool {
+        // FIXME: mention that other checks are done in the query provider
         match target {
             Target::Mod => {}
             _ => {
@@ -1924,53 +1924,7 @@ impl CheckAttrVisitor<'_> {
             }
         }
 
-        let Some(hints) = attr.meta_item_list() else {
-            self.tcx.sess.emit_err(errors::DebugVisualizerInvalid { span: attr.span });
-            return false;
-        };
-
-        let hint = match hints.len() {
-            1 => &hints[0],
-            _ => {
-                self.tcx.sess.emit_err(errors::DebugVisualizerInvalid { span: attr.span });
-                return false;
-            }
-        };
-
-        let Some(meta_item) = hint.meta_item() else {
-            self.tcx.sess.emit_err(errors::DebugVisualizerInvalid { span: attr.span });
-            return false;
-        };
-
-        let visualizer_path = match (meta_item.name_or_empty(), meta_item.value_str()) {
-            (sym::natvis_file, Some(value)) => value,
-            (sym::gdb_script_file, Some(value)) => value,
-            (_, _) => {
-                self.tcx.sess.emit_err(errors::DebugVisualizerInvalid { span: meta_item.span });
-                return false;
-            }
-        };
-
-        let file =
-            match resolve_path(&self.tcx.sess.parse_sess, visualizer_path.as_str(), attr.span) {
-                Ok(file) => file,
-                Err(mut err) => {
-                    err.emit();
-                    return false;
-                }
-            };
-
-        match std::fs::File::open(&file) {
-            Ok(_) => true,
-            Err(error) => {
-                self.tcx.sess.emit_err(errors::DebugVisualizerUnreadable {
-                    span: meta_item.span,
-                    file: &file,
-                    error,
-                });
-                false
-            }
-        }
+        true
     }
 
     /// Outputs an error for `#[allow_internal_unstable]` which can only be applied to macros.
diff --git a/compiler/rustc_passes/src/debugger_visualizer.rs b/compiler/rustc_passes/src/debugger_visualizer.rs
@@ -1,60 +1,64 @@
 //! Detecting usage of the `#[debugger_visualizer]` attribute.
 
-use hir::CRATE_HIR_ID;
-use rustc_data_structures::fx::FxHashSet;
+use rustc_ast::Attribute;
 use rustc_data_structures::sync::Lrc;
 use rustc_expand::base::resolve_path;
-use rustc_hir as hir;
-use rustc_hir::HirId;
-use rustc_middle::query::{LocalCrate, Providers};
-use rustc_middle::ty::TyCtxt;
+use rustc_session::Session;
 use rustc_span::{sym, DebuggerVisualizerFile, DebuggerVisualizerType};
 
-use crate::errors::DebugVisualizerUnreadable;
+use crate::errors::{DebugVisualizerInvalid, DebugVisualizerUnreadable};
 
-fn check_for_debugger_visualizer(
-    tcx: TyCtxt<'_>,
-    hir_id: HirId,
-    debugger_visualizers: &mut FxHashSet<DebuggerVisualizerFile>,
-) {
-    let attrs = tcx.hir().attrs(hir_id);
-    for attr in attrs {
+impl DebuggerVisualizerCollector<'_> {
+    fn check_for_debugger_visualizer(&mut self, attr: &Attribute) {
         if attr.has_name(sym::debugger_visualizer) {
-            let Some(list) = attr.meta_item_list() else {
-                continue
+            let Some(hints) = attr.meta_item_list() else {
+            self.sess.emit_err(DebugVisualizerInvalid { span: attr.span });
+                return;
             };
 
-            let meta_item = match list.len() {
-                1 => match list[0].meta_item() {
-                    Some(meta_item) => meta_item,
-                    _ => continue,
-                },
-                _ => continue,
+            let hint = if hints.len() == 1 {
+                &hints[0]
+            } else {
+                self.sess.emit_err(DebugVisualizerInvalid { span: attr.span });
+                return;
             };
 
-            let visualizer_type = match meta_item.name_or_empty() {
-                sym::natvis_file => DebuggerVisualizerType::Natvis,
-                sym::gdb_script_file => DebuggerVisualizerType::GdbPrettyPrinter,
-                _ => continue,
+            let Some(meta_item) = hint.meta_item() else {
+                self.sess.emit_err(DebugVisualizerInvalid { span: attr.span });
+                return;
             };
 
-            let file = match meta_item.value_str() {
-                Some(value) => {
-                    match resolve_path(&tcx.sess.parse_sess, value.as_str(), attr.span) {
-                        Ok(file) => file,
-                        _ => continue,
+            let (visualizer_type, visualizer_path) =
+                match (meta_item.name_or_empty(), meta_item.value_str()) {
+                    (sym::natvis_file, Some(value)) => (DebuggerVisualizerType::Natvis, value),
+                    (sym::gdb_script_file, Some(value)) => {
+                        (DebuggerVisualizerType::GdbPrettyPrinter, value)
                     }
-                }
-                None => continue,
-            };
+                    (_, _) => {
+                        self.sess.emit_err(DebugVisualizerInvalid { span: meta_item.span });
+                        return;
+                    }
+                };
+
+            let file =
+                match resolve_path(&self.sess.parse_sess, visualizer_path.as_str(), attr.span) {
+                    Ok(file) => file,
+                    Err(mut err) => {
+                        err.emit();
+                        return;
+                    }
+                };
 
             match std::fs::read(&file) {
                 Ok(contents) => {
-                    debugger_visualizers
-                        .insert(DebuggerVisualizerFile::new(Lrc::from(contents), visualizer_type));
+                    self.visualizers.push(DebuggerVisualizerFile::new(
+                        Lrc::from(contents),
+                        visualizer_type,
+                        file,
+                    ));
                 }
                 Err(error) => {
-                    tcx.sess.emit_err(DebugVisualizerUnreadable {
+                    self.sess.emit_err(DebugVisualizerUnreadable {
                         span: meta_item.span,
                         file: &file,
                         error,
@@ -65,31 +69,25 @@ fn check_for_debugger_visualizer(
     }
 }
 
-/// Traverses and collects the debugger visualizers for a specific crate.
-fn debugger_visualizers(tcx: TyCtxt<'_>, _: LocalCrate) -> Vec<DebuggerVisualizerFile> {
-    // Initialize the collector.
-    let mut debugger_visualizers = FxHashSet::default();
-
-    // Collect debugger visualizers in this crate.
-    tcx.hir().for_each_module(|id| {
-        check_for_debugger_visualizer(
-            tcx,
-            tcx.hir().local_def_id_to_hir_id(id),
-            &mut debugger_visualizers,
-        )
-    });
+struct DebuggerVisualizerCollector<'a> {
+    sess: &'a Session,
+    visualizers: Vec<DebuggerVisualizerFile>,
+}
 
-    // Collect debugger visualizers on the crate attributes.
-    check_for_debugger_visualizer(tcx, CRATE_HIR_ID, &mut debugger_visualizers);
+impl<'ast> rustc_ast::visit::Visitor<'ast> for DebuggerVisualizerCollector<'_> {
+    fn visit_attribute(&mut self, attr: &'ast Attribute) {
+        self.check_for_debugger_visualizer(attr);
+        rustc_ast::visit::walk_attribute(self, attr);
+    }
+}
 
-    // Extract out the found debugger_visualizer items.
-    let mut visualizers = debugger_visualizers.into_iter().collect::<Vec<_>>();
+/// Traverses and collects the debugger visualizers for a specific crate.
+pub fn collect(sess: &Session, krate: &rustc_ast::ast::Crate) -> Vec<DebuggerVisualizerFile> {
+    // Initialize the collector.
+    let mut visitor = DebuggerVisualizerCollector { sess, visualizers: Vec::new() };
+    rustc_ast::visit::Visitor::visit_crate(&mut visitor, krate);
 
     // Sort the visualizers so we always get a deterministic query result.
-    visualizers.sort();
-    visualizers
-}
-
-pub fn provide(providers: &mut Providers) {
-    providers.debugger_visualizers = debugger_visualizers;
+    visitor.visualizers.sort_unstable();
+    visitor.visualizers
 }
diff --git a/compiler/rustc_passes/src/lib.rs b/compiler/rustc_passes/src/lib.rs
@@ -27,7 +27,7 @@ use rustc_middle::query::Providers;
 mod check_attr;
 mod check_const;
 pub mod dead;
-mod debugger_visualizer;
+pub mod debugger_visualizer;
 mod diagnostic_items;
 pub mod entry;
 mod errors;
@@ -50,7 +50,6 @@ pub fn provide(providers: &mut Providers) {
     check_attr::provide(providers);
     check_const::provide(providers);
     dead::provide(providers);
-    debugger_visualizer::provide(providers);
     diagnostic_items::provide(providers);
     entry::provide(providers);
     lang_items::provide(providers);
diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs
@@ -1272,11 +1272,21 @@ pub struct DebuggerVisualizerFile {
     pub src: Lrc<[u8]>,
     /// Indicates which visualizer type this targets.
     pub visualizer_type: DebuggerVisualizerType,
+    // FIXME: Docs
+    pub path: Option<PathBuf>,
 }
 
 impl DebuggerVisualizerFile {
-    pub fn new(src: Lrc<[u8]>, visualizer_type: DebuggerVisualizerType) -> Self {
-        DebuggerVisualizerFile { src, visualizer_type }
+    pub fn new(src: Lrc<[u8]>, visualizer_type: DebuggerVisualizerType, path: PathBuf) -> Self {
+        DebuggerVisualizerFile { src, visualizer_type, path: Some(path) }
+    }
+
+    pub fn path_erased(&self) -> Self {
+        DebuggerVisualizerFile {
+            src: self.src.clone(),
+            visualizer_type: self.visualizer_type,
+            path: None,
+        }
     }
 }
 
diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs
@@ -35,6 +35,7 @@ const EXTENSION_EXCEPTION_PATHS: &[&str] = &[
     "tests/ui/macros/syntax-extension-source-utils-files/includeme.fragment", // more include
     "tests/ui/unused-crate-deps/test.mk", // why would you use make
     "tests/ui/proc-macro/auxiliary/included-file.txt", // more include
+    "tests/ui/invalid/foo.natvis.xml", // sample debugger visualizer
 ];
 
 fn check_entries(tests_path: &Path, bad: &mut bool) {
diff --git a/tests/run-make/debugger-visualizer-dep-info/Makefile b/tests/run-make/debugger-visualizer-dep-info/Makefile
@@ -0,0 +1,8 @@
+# ignore-windows-gnu
+
+include ../tools.mk
+
+all:
+	$(RUSTC) --emit dep-info main.rs
+	$(CGREP) "foo.py" < $(TMPDIR)/main.d
+	$(CGREP) "my_visualizers/bar.py" < $(TMPDIR)/main.d
diff --git a/tests/run-make/debugger-visualizer-dep-info/foo.py b/tests/run-make/debugger-visualizer-dep-info/foo.py
@@ -0,0 +1 @@
+# empty
diff --git a/tests/run-make/debugger-visualizer-dep-info/main.rs b/tests/run-make/debugger-visualizer-dep-info/main.rs
@@ -0,0 +1,12 @@
+#![debugger_visualizer(gdb_script_file = "foo.py")]
+
+fn main() {
+    const _UNUSED: u32 = {
+        mod inner {
+            #![debugger_visualizer(gdb_script_file = "my_visualizers/bar.py")]
+            pub const XYZ: u32 = 123;
+        }
+
+        inner::XYZ + 1
+    };
+}
diff --git a/tests/run-make/debugger-visualizer-dep-info/my_visualizers/bar.py b/tests/run-make/debugger-visualizer-dep-info/my_visualizers/bar.py
@@ -0,0 +1 @@
+# empty
diff --git a/tests/ui/invalid/foo.natvis.xml b/tests/ui/invalid/foo.natvis.xml
@@ -0,0 +1 @@
+<!-- empty -->
diff --git a/tests/ui/invalid/invalid-debugger-visualizer-target.rs b/tests/ui/invalid/invalid-debugger-visualizer-target.rs
@@ -1,2 +1,2 @@
-#[debugger_visualizer(natvis_file = "../foo.natvis")] //~ ERROR attribute should be applied to a module
+#[debugger_visualizer(natvis_file = "./foo.natvis.xml")] //~ ERROR attribute should be applied to a module
 fn main() {}
diff --git a/tests/ui/invalid/invalid-debugger-visualizer-target.stderr b/tests/ui/invalid/invalid-debugger-visualizer-target.stderr
@@ -1,8 +1,8 @@
 error: attribute should be applied to a module
   --> $DIR/invalid-debugger-visualizer-target.rs:1:1
    |
-LL | #[debugger_visualizer(natvis_file = "../foo.natvis")]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | #[debugger_visualizer(natvis_file = "./foo.natvis.xml")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

Original file line number	Diff line number	Diff line change
`@@ -1795,6 +1795,7 @@ rustc_queries! {`
`1795`	`1795`	`arena_cache`
`1796`	`1796`	`desc { "looking up the debugger visualizers for this crate" }`
`1797`	`1797`	`separate_provide_extern`
	`1798`	`+ feedable`
`1798`	`1799`	`}`
`1799`	`1800`	`query postorder_cnums(_: ()) -> &'tcx [CrateNum] {`
`1800`	`1801`	`eval_always`