From 4efddb141a37577c6fc6a9a39248cc86b736750b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 10 Jan 2024 10:55:08 +1100 Subject: [PATCH 1/3] Add some helpful comments in `trimmed_def_paths`. To explain things that took me a minute to work out. --- compiler/rustc_middle/src/ty/print/pretty.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index a10bdc6012c07..4d9a1be2087fe 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -3083,7 +3083,8 @@ pub fn trimmed_def_paths(tcx: TyCtxt<'_>, (): ()) -> DefIdMap { let mut map: DefIdMap = Default::default(); if let TrimmedDefPaths::GoodPath = tcx.sess.opts.trimmed_def_paths { - // Trimming paths is expensive and not optimized, since we expect it to only be used for error reporting. + // Trimming paths is expensive and not optimized, since we expect it to only be used for + // error reporting. // // For good paths causing this bug, the `rustc_middle::ty::print::with_no_trimmed_paths` // wrapper can be used to suppress this query, in exchange for full paths being formatted. @@ -3092,6 +3093,8 @@ pub fn trimmed_def_paths(tcx: TyCtxt<'_>, (): ()) -> DefIdMap { ); } + // Once constructed, unique namespace+symbol pairs will have a `Some(_)` entry, while + // non-unique pairs will have a `None` entry. let unique_symbols_rev: &mut FxHashMap<(Namespace, Symbol), Option> = &mut FxHashMap::default(); @@ -3121,6 +3124,7 @@ pub fn trimmed_def_paths(tcx: TyCtxt<'_>, (): ()) -> DefIdMap { } }); + // Put the symbol from all the unique namespace+symbol pairs into `map`. for ((_, symbol), opt_def_id) in unique_symbols_rev.drain() { use std::collections::hash_map::Entry::{Occupied, Vacant}; From 086d17b7cb77850ce9bc549b29082b6938a6c0d4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 10 Jan 2024 11:08:06 +1100 Subject: [PATCH 2/3] Refactor `try_print_trimmed_def_path`. Inverting the condition lets us merge the two `Ok(false)` paths. I also find the inverted condition easier to read: "all the things that must be true for trimming to occur", instead of "any of the things that must be true for trimming to not occur". --- compiler/rustc_middle/src/ty/print/pretty.rs | 29 ++++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 4d9a1be2087fe..3c6ad90b2ea90 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -365,26 +365,19 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write { /// Try to see if this path can be trimmed to a unique symbol name. fn try_print_trimmed_def_path(&mut self, def_id: DefId) -> Result { - if with_forced_trimmed_paths() { - let trimmed = self.force_print_trimmed_def_path(def_id)?; - if trimmed { - return Ok(true); - } + if with_forced_trimmed_paths() && self.force_print_trimmed_def_path(def_id)? { + return Ok(true); } - if !self.tcx().sess.opts.unstable_opts.trim_diagnostic_paths - || matches!(self.tcx().sess.opts.trimmed_def_paths, TrimmedDefPaths::Never) - || with_no_trimmed_paths() - || with_crate_prefix() + if self.tcx().sess.opts.unstable_opts.trim_diagnostic_paths + && !matches!(self.tcx().sess.opts.trimmed_def_paths, TrimmedDefPaths::Never) + && !with_no_trimmed_paths() + && !with_crate_prefix() + && let Some(symbol) = self.tcx().trimmed_def_paths(()).get(&def_id) { - return Ok(false); - } - - match self.tcx().trimmed_def_paths(()).get(&def_id) { - None => Ok(false), - Some(symbol) => { - write!(self, "{}", Ident::with_dummy_span(*symbol))?; - Ok(true) - } + write!(self, "{}", Ident::with_dummy_span(*symbol))?; + Ok(true) + } else { + Ok(false) } } From 32de78cade63ae338bd6e0d47d2f9d7278d7edfc Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 10 Jan 2024 12:47:22 +1100 Subject: [PATCH 3/3] Replace `TrimmedDefPaths` with a bool. It's a tri-state enum but the `Always` variant is never used, so a bool is simpler. --- compiler/rustc_driver_impl/src/lib.rs | 4 ++-- compiler/rustc_middle/src/ty/print/pretty.rs | 24 +++++++++----------- compiler/rustc_session/src/config.rs | 22 +++--------------- compiler/rustc_session/src/options.rs | 2 +- 4 files changed, 17 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 2e4baf26176c3..11dcf4108d4ec 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -35,7 +35,7 @@ use rustc_lint::unerased_lint_store; use rustc_metadata::creader::MetadataLoader; use rustc_metadata::locator; use rustc_session::config::{nightly_options, CG_OPTIONS, Z_OPTIONS}; -use rustc_session::config::{ErrorOutputType, Input, OutFileName, OutputType, TrimmedDefPaths}; +use rustc_session::config::{ErrorOutputType, Input, OutFileName, OutputType}; use rustc_session::getopts::{self, Matches}; use rustc_session::lint::{Lint, LintId}; use rustc_session::{config, EarlyDiagCtxt, Session}; @@ -204,7 +204,7 @@ impl Callbacks for TimePassesCallbacks { // self.time_passes = (config.opts.prints.is_empty() && config.opts.unstable_opts.time_passes) .then(|| config.opts.unstable_opts.time_passes_format); - config.opts.trimmed_def_paths = TrimmedDefPaths::GoodPath; + config.opts.trimmed_def_paths = true; } } diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 3c6ad90b2ea90..6c9000c45f61b 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -15,7 +15,6 @@ use rustc_hir::def::{self, CtorKind, DefKind, Namespace}; use rustc_hir::def_id::{DefIdMap, DefIdSet, ModDefId, CRATE_DEF_ID, LOCAL_CRATE}; use rustc_hir::definitions::{DefKey, DefPathDataName}; use rustc_hir::LangItem; -use rustc_session::config::TrimmedDefPaths; use rustc_session::cstore::{ExternCrate, ExternCrateSource}; use rustc_session::Limit; use rustc_span::symbol::{kw, Ident, Symbol}; @@ -369,7 +368,7 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write { return Ok(true); } if self.tcx().sess.opts.unstable_opts.trim_diagnostic_paths - && !matches!(self.tcx().sess.opts.trimmed_def_paths, TrimmedDefPaths::Never) + && self.tcx().sess.opts.trimmed_def_paths && !with_no_trimmed_paths() && !with_crate_prefix() && let Some(symbol) = self.tcx().trimmed_def_paths(()).get(&def_id) @@ -3073,18 +3072,16 @@ fn for_each_def(tcx: TyCtxt<'_>, mut collect_fn: impl for<'b> FnMut(&'b Ident, N /// See also [`DelayDm`](rustc_error_messages::DelayDm) and [`with_no_trimmed_paths!`]. // this is pub to be able to intra-doc-link it pub fn trimmed_def_paths(tcx: TyCtxt<'_>, (): ()) -> DefIdMap { - let mut map: DefIdMap = Default::default(); + assert!(tcx.sess.opts.trimmed_def_paths); - if let TrimmedDefPaths::GoodPath = tcx.sess.opts.trimmed_def_paths { - // Trimming paths is expensive and not optimized, since we expect it to only be used for - // error reporting. - // - // For good paths causing this bug, the `rustc_middle::ty::print::with_no_trimmed_paths` - // wrapper can be used to suppress this query, in exchange for full paths being formatted. - tcx.sess.good_path_delayed_bug( - "trimmed_def_paths constructed but no error emitted; use `DelayDm` for lints or `with_no_trimmed_paths` for debugging", - ); - } + // Trimming paths is expensive and not optimized, since we expect it to only be used for error + // reporting. + // + // For good paths causing this bug, the `rustc_middle::ty::print::with_no_trimmed_paths` + // wrapper can be used to suppress this query, in exchange for full paths being formatted. + tcx.sess.good_path_delayed_bug( + "trimmed_def_paths constructed but no error emitted; use `DelayDm` for lints or `with_no_trimmed_paths` for debugging", + ); // Once constructed, unique namespace+symbol pairs will have a `Some(_)` entry, while // non-unique pairs will have a `None` entry. @@ -3118,6 +3115,7 @@ pub fn trimmed_def_paths(tcx: TyCtxt<'_>, (): ()) -> DefIdMap { }); // Put the symbol from all the unique namespace+symbol pairs into `map`. + let mut map: DefIdMap = Default::default(); for ((_, symbol), opt_def_id) in unique_symbols_rev.drain() { use std::collections::hash_map::Entry::{Occupied, Vacant}; diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index fe1166457baea..ac15c3b407b27 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -533,21 +533,6 @@ impl Default for ErrorOutputType { } } -/// Parameter to control path trimming. -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] -pub enum TrimmedDefPaths { - /// `try_print_trimmed_def_path` never prints a trimmed path and never calls the expensive - /// query. - #[default] - Never, - /// `try_print_trimmed_def_path` calls the expensive query, the query doesn't call - /// `good_path_delayed_bug`. - Always, - /// `try_print_trimmed_def_path` calls the expensive query, the query calls - /// `good_path_delayed_bug`. - GoodPath, -} - #[derive(Clone, Hash, Debug)] pub enum ResolveDocLinks { /// Do not resolve doc links. @@ -1089,7 +1074,7 @@ impl Default for Options { debug_assertions: true, actually_rustdoc: false, resolve_doc_links: ResolveDocLinks::None, - trimmed_def_paths: TrimmedDefPaths::default(), + trimmed_def_paths: false, cli_forced_codegen_units: None, cli_forced_local_thinlto_off: false, remap_path_prefix: Vec::new(), @@ -2926,7 +2911,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M debug_assertions, actually_rustdoc: false, resolve_doc_links: ResolveDocLinks::ExportedMetadata, - trimmed_def_paths: TrimmedDefPaths::default(), + trimmed_def_paths: false, cli_forced_codegen_units: codegen_units, cli_forced_local_thinlto_off: disable_local_thinlto, remap_path_prefix, @@ -3210,7 +3195,7 @@ pub(crate) mod dep_tracking { LinkerPluginLto, LocationDetail, LtoCli, NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes, Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, - TrimmedDefPaths, WasiExecModel, + WasiExecModel, }; use crate::lint; use crate::utils::NativeLib; @@ -3305,7 +3290,6 @@ pub(crate) mod dep_tracking { SymbolManglingVersion, RemapPathScopeComponents, SourceFileHashAlgorithm, - TrimmedDefPaths, OutFileName, OutputType, RealFileName, diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 2d91a3fbd9116..f4bf79f93f287 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -183,7 +183,7 @@ top_level_options!( resolve_doc_links: ResolveDocLinks [TRACKED], /// Control path trimming. - trimmed_def_paths: TrimmedDefPaths [TRACKED], + trimmed_def_paths: bool [TRACKED], /// Specifications of codegen units / ThinLTO which are forced as a /// result of parsing command line options. These are not necessarily