Skip to content

Commit

Permalink
Avoid some interning in bootstrap
Browse files Browse the repository at this point in the history 
This interning is pointless and only makes the code more complex.

The only remaining use of interning is `TargetSelection`, for which I
left a comment.
  • Loading branch information
@Noratrieb
Noratrieb committed Mar 2, 2024
1 parent 5257aee commit fbb97ed
Show file tree
Hide file tree
Showing 13 changed files with 128 additions and 140 deletions.
24 changes: 11 additions & 13 deletions src/bootstrap/src/core/build_steps/check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,18 @@ use crate::core::builder::{
self, crate_description, Alias, Builder, Kind, RunConfig, ShouldRun, Step,
};
use crate::core::config::TargetSelection;
use crate::utils::cache::Interned;
use crate::INTERNER;
use crate::{Compiler, Mode, Subcommand};
use std::path::{Path, PathBuf};

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct Std {
pub target: TargetSelection,
/// Whether to build only a subset of crates.
///
/// This shouldn't be used from other steps; see the comment on [`compile::Rustc`].
///
/// [`compile::Rustc`]: crate::core::build_steps::compile::Rustc
crates: Interned<Vec<String>>,
crates: Vec<String>,
}

/// Returns args for the subcommand itself (not for cargo)
Expand Down Expand Up @@ -89,7 +87,7 @@ fn cargo_subcommand(kind: Kind) -> &'static str {

impl Std {
pub fn new(target: TargetSelection) -> Self {
Self { target, crates: INTERNER.intern_list(vec![]) }
Self { target, crates: vec![] }
}
}

Expand Down Expand Up @@ -204,15 +202,15 @@ impl Step for Std {
}
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct Rustc {
pub target: TargetSelection,
/// Whether to build only a subset of crates.
///
/// This shouldn't be used from other steps; see the comment on [`compile::Rustc`].
///
/// [`compile::Rustc`]: crate::core::build_steps::compile::Rustc
crates: Interned<Vec<String>>,
crates: Vec<String>,
}

impl Rustc {
Expand All @@ -222,7 +220,7 @@ impl Rustc {
.into_iter()
.map(|krate| krate.name.to_string())
.collect();
Self { target, crates: INTERNER.intern_list(crates) }
Self { target, crates }
}
}

Expand Down Expand Up @@ -305,10 +303,10 @@ impl Step for Rustc {
}
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct CodegenBackend {
pub target: TargetSelection,
pub backend: Interned<String>,
pub backend: &'static str,
}

impl Step for CodegenBackend {
Expand All @@ -321,14 +319,14 @@ impl Step for CodegenBackend {
}

fn make_run(run: RunConfig<'_>) {
for &backend in &[INTERNER.intern_str("cranelift"), INTERNER.intern_str("gcc")] {
for &backend in &["cranelift", "gcc"] {
run.builder.ensure(CodegenBackend { target: run.target, backend });
}
}

fn run(self, builder: &Builder<'_>) {
// FIXME: remove once https://github.com/rust-lang/rust/issues/112393 is resolved
if builder.build.config.vendor && &self.backend == "gcc" {
if builder.build.config.vendor && self.backend == "gcc" {
println!("Skipping checking of `rustc_codegen_gcc` with vendoring enabled.");
return;
}
Expand Down Expand Up @@ -552,7 +550,7 @@ fn codegen_backend_stamp(
builder: &Builder<'_>,
compiler: Compiler,
target: TargetSelection,
backend: Interned<String>,
backend: &str,
) -> PathBuf {
builder
.cargo_out(compiler, Mode::Codegen, target)
Expand Down
5 changes: 2 additions & 3 deletions src/bootstrap/src/core/build_steps/clean.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use std::io::{self, ErrorKind};
use std::path::Path;

use crate::core::builder::{crate_description, Builder, RunConfig, ShouldRun, Step};
use crate::utils::cache::Interned;
use crate::utils::helpers::t;
use crate::{Build, Compiler, Mode, Subcommand};

Expand Down Expand Up @@ -44,10 +43,10 @@ impl Step for CleanAll {

macro_rules! clean_crate_tree {
( $( $name:ident, $mode:path, $root_crate:literal);+ $(;)? ) => { $(
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct $name {
compiler: Compiler,
crates: Interned<Vec<String>>,
crates: Vec<String>,
}

impl Step for $name {
Expand Down
45 changes: 22 additions & 23 deletions src/bootstrap/src/core/build_steps/compile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,21 @@ use crate::core::builder::crate_description;
use crate::core::builder::Cargo;
use crate::core::builder::{Builder, Kind, PathSet, RunConfig, ShouldRun, Step, TaskPath};
use crate::core::config::{DebuginfoLevel, LlvmLibunwind, RustcLto, TargetSelection};
use crate::utils::cache::{Interned, INTERNER};
use crate::utils::helpers::{
exe, get_clang_cl_resource_dir, is_debug_info, is_dylib, output, symlink_dir, t, up_to_date,
};
use crate::LLVM_TOOLS;
use crate::{CLang, Compiler, DependencyType, GitRepo, Mode};
use filetime::FileTime;

#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Std {
pub target: TargetSelection,
pub compiler: Compiler,
/// Whether to build only a subset of crates in the standard library.
///
/// This shouldn't be used from other steps; see the comment on [`Rustc`].
crates: Interned<Vec<String>>,
crates: Vec<String>,
/// When using download-rustc, we need to use a new build of `std` for running unit tests of Std itself,
/// but we need to use the downloaded copy of std for linking to rustdoc. Allow this to be overriden by `builder.ensure` from other steps.
force_recompile: bool,
Expand Down Expand Up @@ -559,13 +558,13 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, car
cargo.rustdocflag("-Zcrate-attr=warn(rust_2018_idioms)");
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct StdLink {
pub compiler: Compiler,
pub target_compiler: Compiler,
pub target: TargetSelection,
/// Not actually used; only present to make sure the cache invalidation is correct.
crates: Interned<Vec<String>>,
crates: Vec<String>,
/// See [`Std::force_recompile`].
force_recompile: bool,
}
Expand Down Expand Up @@ -612,7 +611,7 @@ impl Step for StdLink {
});
let libdir = sysroot.join(lib).join("rustlib").join(target.triple).join("lib");
let hostdir = sysroot.join(lib).join("rustlib").join(compiler.host.triple).join("lib");
(INTERNER.intern_path(libdir), INTERNER.intern_path(hostdir))
(libdir, hostdir)
} else {
let libdir = builder.sysroot_libdir(target_compiler, target);
let hostdir = builder.sysroot_libdir(target_compiler, compiler.host);
Expand Down Expand Up @@ -818,7 +817,7 @@ fn cp_rustc_component_to_ci_sysroot(
}
}

#[derive(Debug, PartialOrd, Ord, Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, PartialOrd, Ord, Clone, PartialEq, Eq, Hash)]
pub struct Rustc {
pub target: TargetSelection,
pub compiler: Compiler,
Expand All @@ -827,7 +826,7 @@ pub struct Rustc {
/// This should only be requested by the user, not used within rustbuild itself.
/// Using it within rustbuild can lead to confusing situation where lints are replayed
/// in two different steps.
crates: Interned<Vec<String>>,
crates: Vec<String>,
}

impl Rustc {
Expand Down Expand Up @@ -1220,13 +1219,13 @@ fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelect
}
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct RustcLink {
pub compiler: Compiler,
pub target_compiler: Compiler,
pub target: TargetSelection,
/// Not actually used; only present to make sure the cache invalidation is correct.
crates: Interned<Vec<String>>,
crates: Vec<String>,
}

impl RustcLink {
Expand Down Expand Up @@ -1261,11 +1260,11 @@ impl Step for RustcLink {
}
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct CodegenBackend {
pub target: TargetSelection,
pub compiler: Compiler,
pub backend: Interned<String>,
pub backend: String,
}

fn needs_codegen_config(run: &RunConfig<'_>) -> bool {
Expand All @@ -1284,7 +1283,7 @@ pub(crate) const CODEGEN_BACKEND_PREFIX: &str = "rustc_codegen_";
fn is_codegen_cfg_needed(path: &TaskPath, run: &RunConfig<'_>) -> bool {
if path.path.to_str().unwrap().contains(CODEGEN_BACKEND_PREFIX) {
let mut needs_codegen_backend_config = true;
for &backend in run.builder.config.codegen_backends(run.target) {
for backend in run.builder.config.codegen_backends(run.target) {
if path
.path
.to_str()
Expand Down Expand Up @@ -1321,15 +1320,15 @@ impl Step for CodegenBackend {
return;
}

for &backend in run.builder.config.codegen_backends(run.target) {
for backend in run.builder.config.codegen_backends(run.target) {
if backend == "llvm" {
continue; // Already built as part of rustc
}

run.builder.ensure(CodegenBackend {
target: run.target,
compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()),
backend,
backend: backend.clone(),
});
}
}
Expand Down Expand Up @@ -1394,7 +1393,7 @@ impl Step for CodegenBackend {
f.display()
);
}
let stamp = codegen_backend_stamp(builder, compiler, target, backend);
let stamp = codegen_backend_stamp(builder, compiler, target, &backend);
let codegen_backend = codegen_backend.to_str().unwrap();
t!(fs::write(stamp, codegen_backend));
}
Expand Down Expand Up @@ -1433,7 +1432,7 @@ fn copy_codegen_backends_to_sysroot(
continue; // Already built as part of rustc
}

let stamp = codegen_backend_stamp(builder, compiler, target, *backend);
let stamp = codegen_backend_stamp(builder, compiler, target, backend);
let dylib = t!(fs::read_to_string(&stamp));
let file = Path::new(&dylib);
let filename = file.file_name().unwrap().to_str().unwrap();
Expand Down Expand Up @@ -1470,7 +1469,7 @@ fn codegen_backend_stamp(
builder: &Builder<'_>,
compiler: Compiler,
target: TargetSelection,
backend: Interned<String>,
backend: &str,
) -> PathBuf {
builder
.cargo_out(compiler, Mode::Codegen, target)
Expand Down Expand Up @@ -1508,7 +1507,7 @@ impl Sysroot {
}

impl Step for Sysroot {
type Output = Interned<PathBuf>;
type Output = PathBuf;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.never()
Expand All @@ -1520,7 +1519,7 @@ impl Step for Sysroot {
/// That is, the sysroot for the stage0 compiler is not what the compiler
/// thinks it is by default, but it's the same as the default for stages
/// 1-3.
fn run(self, builder: &Builder<'_>) -> Interned<PathBuf> {
fn run(self, builder: &Builder<'_>) -> PathBuf {
let compiler = self.compiler;
let host_dir = builder.out.join(compiler.host.triple);

Expand Down Expand Up @@ -1652,7 +1651,7 @@ impl Step for Sysroot {
);
}

INTERNER.intern_path(sysroot)
sysroot
}
}

Expand Down Expand Up @@ -1735,15 +1734,15 @@ impl Step for Assemble {
// to not fail while linking the artifacts.
build_compiler.stage = actual_stage;

for &backend in builder.config.codegen_backends(target_compiler.host) {
for backend in builder.config.codegen_backends(target_compiler.host) {
if backend == "llvm" {
continue; // Already built as part of rustc
}

builder.ensure(CodegenBackend {
compiler: build_compiler,
target: target_compiler.host,
backend,
backend: backend.clone(),
});
}

Expand Down
21 changes: 10 additions & 11 deletions src/bootstrap/src/core/build_steps/dist.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use crate::core::build_steps::llvm;
use crate::core::build_steps::tool::{self, Tool};
use crate::core::builder::{Builder, Kind, RunConfig, ShouldRun, Step};
use crate::core::config::TargetSelection;
use crate::utils::cache::{Interned, INTERNER};
use crate::utils::channel;
use crate::utils::helpers::{exe, is_dylib, output, t, target_supports_cranelift_backend, timeit};
use crate::utils::tarball::{GeneratedTarball, OverlayKind, Tarball};
Expand Down Expand Up @@ -488,8 +487,7 @@ impl Step for Rustc {
}

// Debugger scripts
builder
.ensure(DebuggerScripts { sysroot: INTERNER.intern_path(image.to_owned()), host });
builder.ensure(DebuggerScripts { sysroot: image.to_owned(), host });

// Misc license info
let cp = |file: &str| {
Expand All @@ -503,9 +501,9 @@ impl Step for Rustc {
}
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct DebuggerScripts {
pub sysroot: Interned<PathBuf>,
pub sysroot: PathBuf,
pub host: TargetSelection,
}

Expand Down Expand Up @@ -1263,10 +1261,10 @@ impl Step for Miri {
}
}

#[derive(Debug, PartialOrd, Ord, Copy, Clone, Hash, PartialEq, Eq)]
#[derive(Debug, PartialOrd, Ord, Clone, Hash, PartialEq, Eq)]
pub struct CodegenBackend {
pub compiler: Compiler,
pub backend: Interned<String>,
pub backend: String,
}

impl Step for CodegenBackend {
Expand All @@ -1279,14 +1277,14 @@ impl Step for CodegenBackend {
}

fn make_run(run: RunConfig<'_>) {
for &backend in run.builder.config.codegen_backends(run.target) {
for backend in run.builder.config.codegen_backends(run.target) {
if backend == "llvm" {
continue; // Already built as part of rustc
}

run.builder.ensure(CodegenBackend {
compiler: run.builder.compiler(run.builder.top_stage, run.target),
backend,
backend: backend.clone(),
});
}
}
Expand All @@ -1303,7 +1301,8 @@ impl Step for CodegenBackend {
return None;
}

if !builder.config.codegen_backends(self.compiler.host).contains(&self.backend) {
if !builder.config.codegen_backends(self.compiler.host).contains(&self.backend.to_string())
{
return None;
}

Expand Down Expand Up @@ -1528,7 +1527,7 @@ impl Step for Extended {
add_component!("analysis" => Analysis { compiler, target });
add_component!("rustc-codegen-cranelift" => CodegenBackend {
compiler: builder.compiler(stage, target),
backend: INTERNER.intern_str("cranelift"),
backend: "cranelift".to_string(),
});

let etc = builder.src.join("src/etc/installer");
Expand Down
Loading

0 comments on commit fbb97ed

Please sign in to comment.