Rollup merge of #117376 - nnethercote:rustc_interface-more, r=oli-obk

More `rustc_interface` cleanups

In particular, following up #117268 with more improvement to `--cfg`/`--check-cfg` handling.

r? ``@oli-obk``

compiler/rustc_driver_impl/src/lib.rs

@@ -317,13 +317,11 @@ fn run_compiler(
         return Ok(());
     }
 
-    let cfg = interface::parse_cfg(&early_error_handler, matches.opt_strs("cfg"));
-    let check_cfg = interface::parse_check_cfg(&early_error_handler, matches.opt_strs("check-cfg"));
     let (odir, ofile) = make_output(&matches);
     let mut config = interface::Config {
         opts: sopts,
-        crate_cfg: cfg,
-        crate_check_cfg: check_cfg,
+        crate_cfg: matches.opt_strs("cfg"),
+        crate_check_cfg: matches.opt_strs("check-cfg"),
         input: Input::File(PathBuf::new()),
         output_file: ofile,
         output_dir: odir,

compiler/rustc_interface/src/callbacks.rs

@@ -33,9 +33,7 @@ fn track_diagnostic(diagnostic: &mut Diagnostic, f: &mut dyn FnMut(&mut Diagnost
     tls::with_context_opt(|icx| {
         if let Some(icx) = icx {
             if let Some(diagnostics) = icx.diagnostics {
-                let mut diagnostics = diagnostics.lock();
-                diagnostics.extend(Some(diagnostic.clone()));
-                std::mem::drop(diagnostics);
+                diagnostics.lock().extend(Some(diagnostic.clone()));
             }
 
             // Diagnostics are tracked, we can ignore the dependency.

compiler/rustc_interface/src/passes.rs

@@ -392,34 +392,16 @@ fn generated_output_paths(
     out_filenames
 }
 
-// Runs `f` on every output file path and returns the first non-None result, or None if `f`
-// returns None for every file path.
-fn check_output<F, T>(output_paths: &[PathBuf], f: F) -> Option<T>
-where
-    F: Fn(&PathBuf) -> Option<T>,
-{
-    for output_path in output_paths {
-        if let Some(result) = f(output_path) {
-            return Some(result);
-        }
-    }
-    None
-}
-
 fn output_contains_path(output_paths: &[PathBuf], input_path: &Path) -> bool {
     let input_path = try_canonicalize(input_path).ok();
     if input_path.is_none() {
         return false;
     }
-    let check = |output_path: &PathBuf| {
-        if try_canonicalize(output_path).ok() == input_path { Some(()) } else { None }
-    };
-    check_output(output_paths, check).is_some()
+    output_paths.iter().any(|output_path| try_canonicalize(output_path).ok() == input_path)
 }
 
-fn output_conflicts_with_dir(output_paths: &[PathBuf]) -> Option<PathBuf> {
-    let check = |output_path: &PathBuf| output_path.is_dir().then(|| output_path.clone());
-    check_output(output_paths, check)
+fn output_conflicts_with_dir(output_paths: &[PathBuf]) -> Option<&PathBuf> {
+    output_paths.iter().find(|output_path| output_path.is_dir())
 }
 
 fn escape_dep_filename(filename: &str) -> String {
@@ -602,9 +584,7 @@ fn output_filenames(tcx: TyCtxt<'_>, (): ()) -> Arc<OutputFilenames> {
     let (_, krate) = &*tcx.resolver_for_lowering(()).borrow();
     let crate_name = tcx.crate_name(LOCAL_CRATE);
 
-    // FIXME: rustdoc passes &[] instead of &krate.attrs here
     let outputs = util::build_output_filenames(&krate.attrs, sess);
-
     let output_paths =
         generated_output_paths(tcx, &outputs, sess.io.output_file.is_some(), crate_name);
 
@@ -882,16 +862,18 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
 
             let trait_ref = ty::Binder::dummy(ty::TraitRef::identity(tcx, tr));
 
-            // A slightly edited version of the code in `rustc_trait_selection::traits::vtable::vtable_entries`,
-            // that works without self type and just counts number of entries.
+            // A slightly edited version of the code in
+            // `rustc_trait_selection::traits::vtable::vtable_entries`, that works without self
+            // type and just counts number of entries.
             //
-            // Note that this is technically wrong, for traits which have associated types in supertraits:
+            // Note that this is technically wrong, for traits which have associated types in
+            // supertraits:
             //
             //   trait A: AsRef<Self::T> + AsRef<()> { type T; }
             //
-            // Without self type we can't normalize `Self::T`, so we can't know if `AsRef<Self::T>` and
-            // `AsRef<()>` are the same trait, thus we assume that those are different, and potentially
-            // over-estimate how many vtable entries there are.
+            // Without self type we can't normalize `Self::T`, so we can't know if `AsRef<Self::T>`
+            // and `AsRef<()>` are the same trait, thus we assume that those are different, and
+            // potentially over-estimate how many vtable entries there are.
             //
             // Similarly this is wrong for traits that have methods with possibly-impossible bounds.
             // For example:
@@ -918,10 +900,10 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
                         let own_existential_entries =
                             tcx.own_existential_vtable_entries(trait_ref.def_id());
 
-                        // The original code here ignores the method if its predicates are impossible.
-                        // We can't really do that as, for example, all not trivial bounds on generic
-                        // parameters are impossible (since we don't know the parameters...),
-                        // see the comment above.
+                        // The original code here ignores the method if its predicates are
+                        // impossible. We can't really do that as, for example, all not trivial
+                        // bounds on generic parameters are impossible (since we don't know the
+                        // parameters...), see the comment above.
                         entries_ignoring_upcasting += own_existential_entries.len();
 
                         if emit_vptr {

compiler/rustc_interface/src/tests.rs

@@ -2,43 +2,29 @@
 use crate::interface::parse_cfg;
 use rustc_data_structures::profiling::TimePassesFormat;
 use rustc_errors::{emitter::HumanReadableErrorType, registry, ColorConfig};
-use rustc_session::config::rustc_optgroups;
-use rustc_session::config::Cfg;
-use rustc_session::config::DebugInfo;
-use rustc_session::config::Input;
-use rustc_session::config::InstrumentXRay;
-use rustc_session::config::LinkSelfContained;
-use rustc_session::config::Polonius;
-use rustc_session::config::TraitSolver;
-use rustc_session::config::{build_configuration, build_session_options};
 use rustc_session::config::{
-    BranchProtection, Externs, OomStrategy, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet,
-    ProcMacroExecutionStrategy, SymbolManglingVersion, WasiExecModel,
+    build_configuration, build_session_options, rustc_optgroups, BranchProtection, CFGuard, Cfg,
+    DebugInfo, DumpMonoStatsFormat, ErrorOutputType, ExternEntry, ExternLocation, Externs, Input,
+    InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli,
+    MirSpanview, OomStrategy, Options, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet,
+    Passes, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath, SymbolManglingVersion,
+    TraitSolver, WasiExecModel,
 };
-use rustc_session::config::{CFGuard, ExternEntry, LinkerPluginLto, LtoCli, SwitchWithOptPath};
-use rustc_session::config::{DumpMonoStatsFormat, MirSpanview};
-use rustc_session::config::{ErrorOutputType, ExternLocation, LocationDetail, Options, Strip};
-use rustc_session::config::{InstrumentCoverage, Passes};
 use rustc_session::lint::Level;
 use rustc_session::search_paths::SearchPath;
 use rustc_session::utils::{CanonicalizedPath, NativeLib, NativeLibKind};
-use rustc_session::{build_session, getopts, Session};
-use rustc_session::{CompilerIO, EarlyErrorHandler};
+use rustc_session::{build_session, getopts, CompilerIO, EarlyErrorHandler, Session};
 use rustc_span::edition::{Edition, DEFAULT_EDITION};
 use rustc_span::symbol::sym;
-use rustc_span::FileName;
-use rustc_span::SourceFileHashAlgorithm;
+use rustc_span::{FileName, SourceFileHashAlgorithm};
 use rustc_target::spec::{CodeModel, LinkerFlavorCli, MergeFunctions, PanicStrategy, RelocModel};
 use rustc_target::spec::{RelroLevel, SanitizerSet, SplitDebuginfo, StackProtector, TlsModel};
 use std::collections::{BTreeMap, BTreeSet};
 use std::num::NonZeroUsize;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
-fn mk_session(
-    handler: &mut EarlyErrorHandler,
-    matches: getopts::Matches,
-) -> (Session, Cfg<String>) {
+fn mk_session(handler: &mut EarlyErrorHandler, matches: getopts::Matches) -> (Session, Cfg) {
     let registry = registry::Registry::new(&[]);
     let sessopts = build_session_options(handler, &matches);
     let cfg = parse_cfg(handler, matches.opt_strs("cfg"));

compiler/rustc_interface/src/util.rs

@@ -36,11 +36,7 @@ pub type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
 ///
 /// This is performed by checking whether a set of permitted features
 /// is available on the target machine, by querying the codegen backend.
-pub fn add_configuration(
-    cfg: &mut Cfg<Symbol>,
-    sess: &mut Session,
-    codegen_backend: &dyn CodegenBackend,
-) {
+pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dyn CodegenBackend) {
     let tf = sym::target_feature;
 
     let unstable_target_features = codegen_backend.target_features(sess, true);
@@ -59,8 +55,8 @@ pub fn add_configuration(
 pub fn create_session(
     handler: &EarlyErrorHandler,
     sopts: config::Options,
-    cfg: Cfg<String>,
-    check_cfg: CheckCfg<String>,
+    cfg: Cfg,
+    mut check_cfg: CheckCfg,
     locale_resources: &'static [&'static str],
     file_loader: Option<Box<dyn FileLoader + Send + Sync + 'static>>,
     io: CompilerIO,
@@ -123,7 +119,6 @@ pub fn create_session(
     let mut cfg = config::build_configuration(&sess, cfg);
     add_configuration(&mut cfg, &mut sess, &*codegen_backend);
 
-    let mut check_cfg = check_cfg.intern();
     check_cfg.fill_well_known(&sess.target);
 
     // These configs use symbols, rather than strings.
@@ -487,21 +482,6 @@ fn categorize_crate_type(s: Symbol) -> Option<CrateType> {
 }
 
 pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec<CrateType> {
-    // Unconditionally collect crate types from attributes to make them used
-    let attr_types: Vec<CrateType> = attrs
-        .iter()
-        .filter_map(|a| {
-            if a.has_name(sym::crate_type) {
-                match a.value_str() {
-                    Some(s) => categorize_crate_type(s),
-                    _ => None,
-                }
-            } else {
-                None
-            }
-        })
-        .collect();
-
     // If we're generating a test executable, then ignore all other output
     // styles at all other locations
     if session.opts.test {
@@ -515,6 +495,13 @@ pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec<C
     #[allow(rustc::bad_opt_access)]
     let mut base = session.opts.crate_types.clone();
     if base.is_empty() {
+        let attr_types = attrs.iter().filter_map(|a| {
+            if a.has_name(sym::crate_type) && let Some(s) = a.value_str() {
+                categorize_crate_type(s)
+            } else {
+                None
+            }
+        });
         base.extend(attr_types);
         if base.is_empty() {
             base.push(output::default_output_for_target(session));

compiler/rustc_session/src/config.rs

@@ -1247,8 +1247,8 @@ pub const fn default_lib_output() -> CrateType {
     CrateType::Rlib
 }
 
-fn default_configuration(sess: &Session) -> Cfg<Symbol> {
-    // NOTE: This should be kept in sync with `CheckCfg::<Symbol>::fill_well_known` below.
+fn default_configuration(sess: &Session) -> Cfg {
+    // NOTE: This should be kept in sync with `CheckCfg::fill_well_known` below.
     let end = &sess.target.endian;
     let arch = &sess.target.arch;
     let wordsz = sess.target.pointer_width.to_string();
@@ -1358,56 +1358,21 @@ fn default_configuration(sess: &Session) -> Cfg<Symbol> {
 }
 
 /// The parsed `--cfg` options that define the compilation environment of the
-/// crate, used to drive conditional compilation. `T` is always `String` or
-/// `Symbol`. Strings are used temporarily very early on. Once the the main
-/// symbol interner is running, they are converted to symbols.
+/// crate, used to drive conditional compilation.
 ///
 /// An `FxIndexSet` is used to ensure deterministic ordering of error messages
 /// relating to `--cfg`.
-pub type Cfg<T> = FxIndexSet<(T, Option<T>)>;
+pub type Cfg = FxIndexSet<(Symbol, Option<Symbol>)>;
 
-/// The parsed `--check-cfg` options. The `<T>` structure is similar to `Cfg`.
-pub struct CheckCfg<T> {
+/// The parsed `--check-cfg` options.
+#[derive(Default)]
+pub struct CheckCfg {
     /// Is well known names activated
     pub exhaustive_names: bool,
     /// Is well known values activated
     pub exhaustive_values: bool,
     /// All the expected values for a config name
-    pub expecteds: FxHashMap<T, ExpectedValues<T>>,
-}
-
-impl<T> Default for CheckCfg<T> {
-    fn default() -> Self {
-        CheckCfg {
-            exhaustive_names: false,
-            exhaustive_values: false,
-            expecteds: FxHashMap::default(),
-        }
-    }
-}
-
-impl CheckCfg<String> {
-    pub fn intern(self) -> CheckCfg<Symbol> {
-        CheckCfg {
-            exhaustive_names: self.exhaustive_names,
-            exhaustive_values: self.exhaustive_values,
-            expecteds: self
-                .expecteds
-                .into_iter()
-                .map(|(name, values)| {
-                    (
-                        Symbol::intern(&name),
-                        match values {
-                            ExpectedValues::Some(values) => ExpectedValues::Some(
-                                values.into_iter().map(|b| b.map(|b| Symbol::intern(&b))).collect(),
-                            ),
-                            ExpectedValues::Any => ExpectedValues::Any,
-                        },
-                    )
-                })
-                .collect(),
-        }
-    }
+    pub expecteds: FxHashMap<Symbol, ExpectedValues<Symbol>>,
 }
 
 pub enum ExpectedValues<T> {
@@ -1442,7 +1407,7 @@ impl<'a, T: Eq + Hash + Copy + 'a> Extend<&'a T> for ExpectedValues<T> {
     }
 }
 
-impl CheckCfg<Symbol> {
+impl CheckCfg {
     pub fn fill_well_known(&mut self, current_target: &Target) {
         if !self.exhaustive_values && !self.exhaustive_names {
             return;
@@ -1582,13 +1547,7 @@ impl CheckCfg<Symbol> {
     }
 }
 
-pub fn build_configuration(sess: &Session, user_cfg: Cfg<String>) -> Cfg<Symbol> {
-    // We can now intern these strings.
-    let mut user_cfg: Cfg<Symbol> = user_cfg
-        .into_iter()
-        .map(|(a, b)| (Symbol::intern(&a), b.map(|b| Symbol::intern(&b))))
-        .collect();
-
+pub fn build_configuration(sess: &Session, mut user_cfg: Cfg) -> Cfg {
     // Combine the configuration requested by the session (command line) with
     // some default and generated configuration items.
     let default_cfg = default_configuration(sess);

compiler/rustc_session/src/parse.rs

@@ -188,8 +188,8 @@ pub fn add_feature_diagnostics_for_issue(
 pub struct ParseSess {
     pub span_diagnostic: Handler,
     pub unstable_features: UnstableFeatures,
-    pub config: Cfg<Symbol>,
-    pub check_config: CheckCfg<Symbol>,
+    pub config: Cfg,
+    pub check_config: CheckCfg,
     pub edition: Edition,
     /// Places where raw identifiers were used. This is used to avoid complaining about idents
     /// clashing with keywords in new editions.

src/librustdoc/core.rs

@@ -14,8 +14,8 @@ use rustc_lint::{late_lint_mod, MissingDoc};
 use rustc_middle::hir::nested_filter;
 use rustc_middle::ty::{ParamEnv, Ty, TyCtxt};
 use rustc_session::config::{self, CrateType, ErrorOutputType, ResolveDocLinks};
+use rustc_session::lint;
 use rustc_session::Session;
-use rustc_session::{lint, EarlyErrorHandler};
 use rustc_span::symbol::sym;
 use rustc_span::{source_map, Span};
 
@@ -175,7 +175,6 @@ pub(crate) fn new_handler(
 
 /// Parse, resolve, and typecheck the given crate.
 pub(crate) fn create_config(
-    handler: &EarlyErrorHandler,
     RustdocOptions {
         input,
         crate_name,
@@ -255,8 +254,8 @@ pub(crate) fn create_config(
 
     interface::Config {
         opts: sessopts,
-        crate_cfg: interface::parse_cfg(handler, cfgs),
-        crate_check_cfg: interface::parse_check_cfg(handler, check_cfgs),
+        crate_cfg: cfgs,
+        crate_check_cfg: check_cfgs,
         input,
         output_file: None,
         output_dir: None,