From 7f01893900cf01b48adcaa0146e139cc8083b399 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 16 May 2023 13:37:46 +0200 Subject: [PATCH] Fix dependency tracking for debugger visualizers --- compiler/rustc_interface/src/passes.rs | 11 ++ compiler/rustc_metadata/src/rmeta/encoder.rs | 11 +- compiler/rustc_middle/src/hir/map/mod.rs | 12 +- compiler/rustc_middle/src/query/mod.rs | 1 + compiler/rustc_passes/src/check_attr.rs | 50 +------- .../rustc_passes/src/debugger_visualizer.rs | 118 +++++++++--------- compiler/rustc_passes/src/lib.rs | 3 +- compiler/rustc_span/src/lib.rs | 14 ++- src/tools/tidy/src/ui_tests.rs | 1 + .../debugger-visualizer-dep-info/Makefile | 8 ++ .../debugger-visualizer-dep-info/foo.py | 1 + .../debugger-visualizer-dep-info/main.rs | 12 ++ .../my_visualizers/bar.py | 1 + tests/ui/invalid/foo.natvis.xml | 1 + .../invalid-debugger-visualizer-target.rs | 2 +- .../invalid-debugger-visualizer-target.stderr | 4 +- 16 files changed, 133 insertions(+), 117 deletions(-) create mode 100644 tests/run-make/debugger-visualizer-dep-info/Makefile create mode 100644 tests/run-make/debugger-visualizer-dep-info/foo.py create mode 100644 tests/run-make/debugger-visualizer-dep-info/main.rs create mode 100644 tests/run-make/debugger-visualizer-dep-info/my_visualizers/bar.py create mode 100644 tests/ui/invalid/foo.natvis.xml diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index cb10916abb1..66bdc9d2a3e 100644 --- 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 index c1815ae3851..7eab0703165 100644 --- 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 { 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 { diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 5bf0938d518..effa38dc2dc 100644 --- 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 index ef13a277207..3bf62db3135 100644 --- 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 index 455d7b89f9c..a148ae78630 100644 --- 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 index 8ea95b3f383..d75258c0511 100644 --- 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, -) { - 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( } } +struct DebuggerVisualizerCollector<'a> { + sess: &'a Session, + visualizers: Vec, +} + +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); + } +} + /// Traverses and collects the debugger visualizers for a specific crate. -fn debugger_visualizers(tcx: TyCtxt<'_>, _: LocalCrate) -> Vec { +pub fn collect(sess: &Session, krate: &rustc_ast::ast::Crate) -> Vec { // 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, - ) - }); - - // Collect debugger visualizers on the crate attributes. - check_for_debugger_visualizer(tcx, CRATE_HIR_ID, &mut debugger_visualizers); - - // Extract out the found debugger_visualizer items. - let mut visualizers = debugger_visualizers.into_iter().collect::>(); + 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 index 0da4b294648..5b1ed7f3fa4 100644 --- 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 index 97cb734619e..9ded311d15d 100644 --- 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, } 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 index ee12f4acb10..be3a5d3aa0f 100644 --- 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 new file mode 100644 index 00000000000..0a2493144ea --- /dev/null +++ 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 new file mode 100644 index 00000000000..1bb8bf6d7fd --- /dev/null +++ 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 new file mode 100644 index 00000000000..6caf2afe9a5 --- /dev/null +++ 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 new file mode 100644 index 00000000000..1bb8bf6d7fd --- /dev/null +++ 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 new file mode 100644 index 00000000000..c341a403902 --- /dev/null +++ b/tests/ui/invalid/foo.natvis.xml @@ -0,0 +1 @@ + diff --git a/tests/ui/invalid/invalid-debugger-visualizer-target.rs b/tests/ui/invalid/invalid-debugger-visualizer-target.rs index f9dd20dbfed..1efb9555c24 100644 --- 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 index 7944f751859..c8a4d681379 100644 --- 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