From ee8e0bc5824be2a05f086649815b2e10473663ea Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 19 Jun 2022 17:25:20 -0500 Subject: [PATCH 01/10] Only call default steps once, not once for each PathSet Running steps multiple times defeats the whole point of #96501, since lint messages will be duplicated. --- src/bootstrap/builder.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 62b5416cee8af..d58829721cc9a 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -304,9 +304,7 @@ impl StepDescription { if paths.is_empty() || builder.config.include_default_paths { for (desc, should_run) in v.iter().zip(&should_runs) { if desc.default && should_run.is_really_default() { - for pathset in &should_run.paths { - desc.maybe_run(builder, vec![pathset.clone()]); - } + desc.maybe_run(builder, should_run.paths.iter().cloned().collect()); } } } From 0566ade0b13fa5e4e09ea6d72fe7cb92a1cb13e3 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 26 Jun 2022 21:06:24 -0500 Subject: [PATCH 02/10] Use generics for interned types rather than copy-pasting impls This makes it much simpler to add new interned types, rather than having to add 4+ impl blocks for each type. --- src/bootstrap/cache.rs | 112 ++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 69 deletions(-) diff --git a/src/bootstrap/cache.rs b/src/bootstrap/cache.rs index 97f0bfdc484da..72de0a090af2d 100644 --- a/src/bootstrap/cache.rs +++ b/src/bootstrap/cache.rs @@ -4,13 +4,12 @@ use std::cell::RefCell; use std::cmp::{Ord, Ordering, PartialOrd}; use std::collections::HashMap; use std::convert::AsRef; -use std::ffi::OsStr; use std::fmt; use std::hash::{Hash, Hasher}; use std::marker::PhantomData; use std::mem; use std::ops::Deref; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::sync::Mutex; // FIXME: replace with std::lazy after it gets stabilized and reaches beta @@ -20,15 +19,9 @@ use crate::builder::Step; pub struct Interned(usize, PhantomData<*const T>); -impl Default for Interned { +impl Default for Interned { fn default() -> Self { - INTERNER.intern_string(String::default()) - } -} - -impl Default for Interned { - fn default() -> Self { - INTERNER.intern_path(PathBuf::default()) + T::default().intern() } } @@ -77,87 +70,48 @@ impl fmt::Display for Interned { } } -impl fmt::Debug for Interned { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let s: &str = &*self; - f.write_fmt(format_args!("{:?}", s)) - } -} -impl fmt::Debug for Interned { +impl fmt::Debug for Interned +where + Self: Deref, +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let s: &Path = &*self; + let s: &U = &*self; f.write_fmt(format_args!("{:?}", s)) } } -impl Hash for Interned { +impl Hash for Interned { fn hash(&self, state: &mut H) { - let l = INTERNER.strs.lock().unwrap(); + let l = T::intern_cache().lock().unwrap(); l.get(*self).hash(state) } } -impl Hash for Interned { - fn hash(&self, state: &mut H) { - let l = INTERNER.paths.lock().unwrap(); - l.get(*self).hash(state) - } -} - -impl Deref for Interned { - type Target = str; - fn deref(&self) -> &'static str { - let l = INTERNER.strs.lock().unwrap(); - unsafe { mem::transmute::<&str, &'static str>(l.get(*self)) } - } -} - -impl Deref for Interned { - type Target = Path; - fn deref(&self) -> &'static Path { - let l = INTERNER.paths.lock().unwrap(); - unsafe { mem::transmute::<&Path, &'static Path>(l.get(*self)) } - } -} - -impl AsRef for Interned { - fn as_ref(&self) -> &'static Path { - let l = INTERNER.paths.lock().unwrap(); - unsafe { mem::transmute::<&Path, &'static Path>(l.get(*self)) } - } -} - -impl AsRef for Interned { - fn as_ref(&self) -> &'static Path { - let l = INTERNER.strs.lock().unwrap(); - unsafe { mem::transmute::<&Path, &'static Path>(l.get(*self).as_ref()) } +impl Deref for Interned { + type Target = T::Target; + fn deref(&self) -> &'static Self::Target { + let l = T::intern_cache().lock().unwrap(); + unsafe { mem::transmute::<&Self::Target, &'static Self::Target>(l.get(*self)) } } } -impl AsRef for Interned { - fn as_ref(&self) -> &'static OsStr { - let l = INTERNER.paths.lock().unwrap(); - unsafe { mem::transmute::<&OsStr, &'static OsStr>(l.get(*self).as_ref()) } +impl, U: ?Sized> AsRef for Interned { + fn as_ref(&self) -> &'static U { + let l = T::intern_cache().lock().unwrap(); + unsafe { mem::transmute::<&U, &'static U>(l.get(*self).as_ref()) } } } -impl AsRef for Interned { - fn as_ref(&self) -> &'static OsStr { - let l = INTERNER.strs.lock().unwrap(); - unsafe { mem::transmute::<&OsStr, &'static OsStr>(l.get(*self).as_ref()) } - } -} - -impl PartialOrd> for Interned { +impl PartialOrd for Interned { fn partial_cmp(&self, other: &Self) -> Option { - let l = INTERNER.strs.lock().unwrap(); + let l = T::intern_cache().lock().unwrap(); l.get(*self).partial_cmp(l.get(*other)) } } -impl Ord for Interned { +impl Ord for Interned { fn cmp(&self, other: &Self) -> Ordering { - let l = INTERNER.strs.lock().unwrap(); + let l = T::intern_cache().lock().unwrap(); l.get(*self).cmp(l.get(*other)) } } @@ -210,6 +164,26 @@ pub struct Interner { paths: Mutex>, } +trait Internable: Clone + Eq + Hash + 'static { + fn intern_cache() -> &'static Mutex>; + + fn intern(self) -> Interned { + Self::intern_cache().lock().unwrap().intern(self) + } +} + +impl Internable for String { + fn intern_cache() -> &'static Mutex> { + &INTERNER.strs + } +} + +impl Internable for PathBuf { + fn intern_cache() -> &'static Mutex> { + &INTERNER.paths + } +} + impl Interner { pub fn intern_str(&self, s: &str) -> Interned { self.strs.lock().unwrap().intern_borrow(s) From d0011b0c057d39dd9a6a1a671a22aad43b38535c Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 26 Jun 2022 21:07:27 -0500 Subject: [PATCH 03/10] Allow building single crates for the compiler and standard library - Add `Interned>` and use it for tail args - Refactor `cache.rs` not to need a separate impl for each internable type --- src/bootstrap/builder.rs | 15 +++- src/bootstrap/builder/tests.rs | 125 +++++++++++++++++---------------- src/bootstrap/cache.rs | 11 +++ src/bootstrap/check.rs | 4 +- src/bootstrap/compile.rs | 77 ++++++++++++++++---- src/bootstrap/dist.rs | 6 +- src/bootstrap/doc.rs | 12 ++-- src/bootstrap/test.rs | 32 ++++----- src/bootstrap/tool.rs | 10 +-- 9 files changed, 182 insertions(+), 110 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index d58829721cc9a..49f4847c7c767 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -13,7 +13,6 @@ use std::process::{Command, Stdio}; use std::time::{Duration, Instant}; use crate::cache::{Cache, Interned, INTERNER}; -use crate::compile; use crate::config::{SplitDebuginfo, TargetSelection}; use crate::dist; use crate::doc; @@ -26,6 +25,7 @@ use crate::tool::{self, SourceType}; use crate::util::{self, add_dylib_path, add_link_lib_path, exe, libdir, output, t}; use crate::EXTRA_CHECK_CFGS; use crate::{check, Config}; +use crate::{compile, Crate}; use crate::{Build, CLang, DocTests, GitRepo, Mode}; pub use crate::Compiler; @@ -422,8 +422,16 @@ impl<'a> ShouldRun<'a> { /// any of its (local) dependencies. /// /// `make_run` will be called a single time with all matching command-line paths. - pub fn krate(mut self, name: &str) -> Self { - for krate in self.builder.in_tree_crates(name, None) { + pub fn crate_or_deps(self, name: &str) -> Self { + let crates = self.builder.in_tree_crates(name, None); + self.crates(crates) + } + + /// Indicates it should run if the command-line selects any of the given crates. + /// + /// `make_run` will be called a single time with all matching command-line paths. + pub(crate) fn crates(mut self, crates: Vec<&Crate>) -> Self { + for krate in crates { let path = krate.local_path(self.builder); self.paths.insert(PathSet::one(path, self.kind)); } @@ -579,6 +587,7 @@ impl<'a> Builder<'a> { match kind { Kind::Build => describe!( compile::Std, + compile::Rustc, compile::Assemble, compile::CodegenBackend, compile::StartupObjects, diff --git a/src/bootstrap/builder/tests.rs b/src/bootstrap/builder/tests.rs index 70cb0de7cce04..c084e77d3a994 100644 --- a/src/bootstrap/builder/tests.rs +++ b/src/bootstrap/builder/tests.rs @@ -57,6 +57,24 @@ fn check_cli(paths: [&str; N]) { ); } +macro_rules! std { + ($host:ident => $target:ident, stage = $stage:literal) => { + compile::Std::new( + Compiler { host: TargetSelection::from_user(stringify!($host)), stage: $stage }, + TargetSelection::from_user(stringify!($target)), + ) + }; +} + +macro_rules! rustc { + ($host:ident => $target:ident, stage = $stage:literal) => { + compile::Rustc::new( + Compiler { host: TargetSelection::from_user(stringify!($host)), stage: $stage }, + TargetSelection::from_user(stringify!($target)), + ) + }; +} + #[test] fn test_valid() { // make sure multi suite paths are accepted @@ -117,6 +135,17 @@ fn test_exclude_kind() { assert!(run_build(&[path], config).contains::()); } +/// Ensure that if someone passes both a single crate and `library`, all library crates get built. +#[test] +fn alias_and_path_for_library() { + let mut cache = + run_build(&["library".into(), "core".into()], configure("build", &["A"], &["A"])); + assert_eq!( + first(cache.all::()), + &[std!(A => A, stage = 0), std!(A => A, stage = 1)] + ); +} + mod defaults { use super::{configure, first, run_build}; use crate::builder::*; @@ -130,10 +159,7 @@ mod defaults { let a = TargetSelection::from_user("A"); assert_eq!( first(cache.all::()), - &[ - compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a }, - ] + &[std!(A => A, stage = 0), std!(A => A, stage = 1),] ); assert!(!cache.all::().is_empty()); // Make sure rustdoc is only built once. @@ -143,10 +169,7 @@ mod defaults { // - this is the compiler it's _linked_ to, not built with. &[tool::Rustdoc { compiler: Compiler { host: a, stage: 1 } }], ); - assert_eq!( - first(cache.all::()), - &[compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: a },] - ); + assert_eq!(first(cache.all::()), &[rustc!(A => A, stage = 0)],); } #[test] @@ -155,10 +178,7 @@ mod defaults { let mut cache = run_build(&[], config); let a = TargetSelection::from_user("A"); - assert_eq!( - first(cache.all::()), - &[compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a },] - ); + assert_eq!(first(cache.all::()), &[std!(A => A, stage = 0)]); assert!(!cache.all::().is_empty()); assert_eq!( first(cache.all::()), @@ -185,10 +205,10 @@ mod defaults { assert_eq!( first(cache.all::()), &[ - compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 0 }, target: b }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: b }, + std!(A => A, stage = 0), + std!(A => A, stage = 1), + std!(A => B, stage = 0), + std!(A => B, stage = 1), ] ); assert_eq!( @@ -208,10 +228,7 @@ mod defaults { ); assert_eq!( first(cache.all::()), - &[ - compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: b }, - ] + &[rustc!(A => A, stage = 0), rustc!(A => B, stage = 0),] ); } @@ -334,11 +351,11 @@ mod dist { assert_eq!( first(cache.all::()), &[ - compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 2 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: b }, - compile::Std { compiler: Compiler { host: a, stage: 2 }, target: b }, + std!(A => A, stage = 0), + std!(A => A, stage = 1), + std!(A => A, stage = 2), + std!(A => B, stage = 1), + std!(A => B, stage = 2), ], ); assert_eq!(first(cache.all::()), &[dist::Src]); @@ -346,7 +363,6 @@ mod dist { #[test] fn dist_only_cross_host() { - let a = TargetSelection::from_user("A"); let b = TargetSelection::from_user("B"); let mut config = configure(&["A", "B"], &["A", "B"]); config.docs = false; @@ -360,10 +376,7 @@ mod dist { ); assert_eq!( first(cache.all::()), - &[ - compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Rustc { compiler: Compiler { host: a, stage: 1 }, target: b }, - ] + &[rustc!(A => A, stage = 0), rustc!(A => B, stage = 1),] ); } @@ -450,11 +463,11 @@ mod dist { assert_eq!( first(cache.all::()), &[ - compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 2 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: b }, - compile::Std { compiler: Compiler { host: a, stage: 2 }, target: b }, + std!(A => A, stage = 0), + std!(A => A, stage = 1), + std!(A => A, stage = 2), + std!(A => B, stage = 1), + std!(A => B, stage = 2), ] ); assert_eq!( @@ -474,33 +487,29 @@ mod dist { let mut builder = Builder::new(&build); builder.run_step_descriptions( &Builder::get_step_descriptions(Kind::Build), - &["compiler/rustc".into(), "library/std".into()], + &["compiler/rustc".into(), "library".into()], ); - let a = TargetSelection::from_user("A"); - let b = TargetSelection::from_user("B"); - let c = TargetSelection::from_user("C"); - assert_eq!( first(builder.cache.all::()), &[ - compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 2 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: b }, - compile::Std { compiler: Compiler { host: a, stage: 2 }, target: b }, - compile::Std { compiler: Compiler { host: a, stage: 2 }, target: c }, + std!(A => A, stage = 0), + std!(A => A, stage = 1), + std!(A => A, stage = 2), + std!(A => B, stage = 1), + std!(A => B, stage = 2), + std!(A => C, stage = 2), ] ); - assert!(!builder.cache.all::().is_empty()); + assert_eq!(builder.cache.all::().len(), 5); assert_eq!( first(builder.cache.all::()), &[ - compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Rustc { compiler: Compiler { host: a, stage: 1 }, target: a }, - compile::Rustc { compiler: Compiler { host: a, stage: 2 }, target: a }, - compile::Rustc { compiler: Compiler { host: a, stage: 1 }, target: b }, - compile::Rustc { compiler: Compiler { host: a, stage: 2 }, target: b }, + rustc!(A => A, stage = 0), + rustc!(A => A, stage = 1), + rustc!(A => A, stage = 2), + rustc!(A => B, stage = 1), + rustc!(A => B, stage = 2), ] ); } @@ -513,15 +522,10 @@ mod dist { builder.run_step_descriptions(&Builder::get_step_descriptions(Kind::Build), &[]); let a = TargetSelection::from_user("A"); - let c = TargetSelection::from_user("C"); assert_eq!( first(builder.cache.all::()), - &[ - compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a }, - compile::Std { compiler: Compiler { host: a, stage: 2 }, target: c }, - ] + &[std!(A => A, stage = 0), std!(A => A, stage = 1), std!(A => C, stage = 2),] ); assert_eq!( first(builder.cache.all::()), @@ -533,10 +537,7 @@ mod dist { ); assert_eq!( first(builder.cache.all::()), - &[ - compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: a }, - compile::Rustc { compiler: Compiler { host: a, stage: 1 }, target: a }, - ] + &[rustc!(A => A, stage = 0), rustc!(A => A, stage = 1),] ); } diff --git a/src/bootstrap/cache.rs b/src/bootstrap/cache.rs index 72de0a090af2d..be5c9bb078808 100644 --- a/src/bootstrap/cache.rs +++ b/src/bootstrap/cache.rs @@ -162,6 +162,7 @@ impl TyIntern { pub struct Interner { strs: Mutex>, paths: Mutex>, + lists: Mutex>>, } trait Internable: Clone + Eq + Hash + 'static { @@ -184,6 +185,12 @@ impl Internable for PathBuf { } } +impl Internable for Vec { + fn intern_cache() -> &'static Mutex> { + &INTERNER.lists + } +} + impl Interner { pub fn intern_str(&self, s: &str) -> Interned { self.strs.lock().unwrap().intern_borrow(s) @@ -195,6 +202,10 @@ impl Interner { pub fn intern_path(&self, s: PathBuf) -> Interned { self.paths.lock().unwrap().intern(s) } + + pub fn intern_list(&self, v: Vec) -> Interned> { + self.lists.lock().unwrap().intern(v) + } } pub static INTERNER: Lazy = Lazy::new(Interner::default); diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 731ebc41bb9a0..4985b0546789d 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -184,8 +184,8 @@ impl Step for Rustc { // the sysroot for the compiler to find. Otherwise, we're going to // fail when building crates that need to generate code (e.g., build // scripts and their dependencies). - builder.ensure(crate::compile::Std { target: compiler.host, compiler }); - builder.ensure(crate::compile::Std { target, compiler }); + builder.ensure(crate::compile::Std::new(compiler, compiler.host)); + builder.ensure(crate::compile::Std::new(compiler, target)); } else { builder.ensure(Std { target }); } diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index b4807d1ab3af2..399be26d5ac14 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -29,10 +29,31 @@ use crate::util::{exe, is_debug_info, is_dylib, output, symlink_dir, t, up_to_da use crate::LLVM_TOOLS; use crate::{CLang, Compiler, DependencyType, GitRepo, Mode}; -#[derive(Debug, PartialOrd, Ord, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, 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>, +} + +impl Std { + pub fn new(compiler: Compiler, target: TargetSelection) -> Self { + Self { target, compiler, crates: Default::default() } + } +} + +/// Return a `-p=x -p=y` string suitable for passing to a cargo invocation. +fn build_crates_in_set(run: &RunConfig<'_>) -> Interned> { + let mut crates = Vec::new(); + for krate in &run.paths { + let path = krate.assert_single_path(); + let crate_name = run.builder.crate_paths[&path.path]; + crates.push(format!("-p={crate_name}")); + } + INTERNER.intern_list(crates) } impl Step for Std { @@ -43,15 +64,22 @@ impl Step for Std { // When downloading stage1, the standard library has already been copied to the sysroot, so // there's no need to rebuild it. let builder = run.builder; - run.all_krates("test") + run.crate_or_deps("test") .path("library") .lazy_default_condition(Box::new(|| !builder.download_rustc())) } fn make_run(run: RunConfig<'_>) { + // Normally, people will pass *just* library if they pass it. + // But it's possible (although strange) to pass something like `library std core`. + // Build all crates anyway, as if they hadn't passed the other args. + let has_library = + run.paths.iter().any(|set| set.assert_single_path().path.ends_with("library")); + let crates = if has_library { Default::default() } else { build_crates_in_set(&run) }; run.builder.ensure(Std { compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()), target: run.target, + crates, }); } @@ -86,7 +114,7 @@ impl Step for Std { let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target); if compiler_to_use != compiler { - builder.ensure(Std { compiler: compiler_to_use, target }); + builder.ensure(Std::new(compiler_to_use, target)); builder.info(&format!("Uplifting stage1 std ({} -> {})", compiler_to_use.host, target)); // Even if we're not building std this stage, the new sysroot must @@ -115,7 +143,7 @@ impl Step for Std { run_cargo( builder, cargo, - vec![], + self.crates.to_vec(), &libstd_stamp(builder, compiler, target), target_deps, false, @@ -524,6 +552,18 @@ impl Step for StartupObjects { pub struct Rustc { pub target: TargetSelection, pub compiler: Compiler, + /// Whether to build a subset of crates, rather than the whole compiler. + /// + /// 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>, +} + +impl Rustc { + pub fn new(compiler: Compiler, target: TargetSelection) -> Self { + Self { target, compiler, crates: Default::default() } + } } impl Step for Rustc { @@ -532,13 +572,22 @@ impl Step for Rustc { const DEFAULT: bool = false; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() + let mut crates = run.builder.in_tree_crates("rustc-main", None); + for (i, krate) in crates.iter().enumerate() { + if krate.name == "rustc-main" { + crates.swap_remove(i); + break; + } + } + run.crates(crates) } fn make_run(run: RunConfig<'_>) { + let crates = build_crates_in_set(&run); run.builder.ensure(Rustc { compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()), target: run.target, + crates, }); } @@ -560,7 +609,7 @@ impl Step for Rustc { return; } - builder.ensure(Std { compiler, target }); + builder.ensure(Std::new(compiler, target)); if builder.config.keep_stage.contains(&compiler.stage) { builder.info("Warning: Using a potentially old librustc. This may not behave well."); @@ -571,7 +620,7 @@ impl Step for Rustc { let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target); if compiler_to_use != compiler { - builder.ensure(Rustc { compiler: compiler_to_use, target }); + builder.ensure(Rustc::new(compiler_to_use, target)); builder .info(&format!("Uplifting stage1 rustc ({} -> {})", builder.config.build, target)); builder.ensure(RustcLink { @@ -583,10 +632,10 @@ impl Step for Rustc { } // Ensure that build scripts and proc macros have a std / libproc_macro to link against. - builder.ensure(Std { - compiler: builder.compiler(self.compiler.stage, builder.config.build), - target: builder.config.build, - }); + builder.ensure(Std::new( + builder.compiler(self.compiler.stage, builder.config.build), + builder.config.build, + )); let mut cargo = builder.cargo(compiler, Mode::Rustc, SourceType::InTree, target, "build"); rustc_cargo(builder, &mut cargo, target); @@ -633,7 +682,7 @@ impl Step for Rustc { run_cargo( builder, cargo, - vec![], + self.crates.to_vec(), &librustc_stamp(builder, compiler, target), vec![], false, @@ -821,7 +870,7 @@ impl Step for CodegenBackend { let target = self.target; let backend = self.backend; - builder.ensure(Rustc { compiler, target }); + builder.ensure(Rustc::new(compiler, target)); if builder.config.keep_stage.contains(&compiler.stage) { builder.info( @@ -1103,7 +1152,7 @@ impl Step for Assemble { // link to these. (FIXME: Is that correct? It seems to be correct most // of the time but I think we do link to these for stage2/bin compilers // when not performing a full bootstrap). - builder.ensure(Rustc { compiler: build_compiler, target: target_compiler.host }); + builder.ensure(Rustc::new(build_compiler, target_compiler.host)); for &backend in builder.config.rust_codegen_backends.iter() { if backend == "llvm" { diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs index 8182d2bf8fb3b..65ad32670d841 100644 --- a/src/bootstrap/dist.rs +++ b/src/bootstrap/dist.rs @@ -557,7 +557,7 @@ impl Step for Std { return None; } - builder.ensure(compile::Std { compiler, target }); + builder.ensure(compile::Std::new(compiler, target)); let mut tarball = Tarball::new(builder, "rust-std", &target.triple); tarball.include_target_in_component_name(true); @@ -603,7 +603,7 @@ impl Step for RustcDev { return None; } - builder.ensure(compile::Rustc { compiler, target }); + builder.ensure(compile::Rustc::new(compiler, target)); let tarball = Tarball::new(builder, "rustc-dev", &target.triple); @@ -666,7 +666,7 @@ impl Step for Analysis { return None; } - builder.ensure(compile::Std { compiler, target }); + builder.ensure(compile::Std::new(compiler, target)); let src = builder .stage_out(compiler, Mode::Std) .join(target.triple) diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index be6655ddb61d0..59c5651acdb91 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -534,7 +534,9 @@ impl Step for Rustc { fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { let builder = run.builder; - run.krate("rustc-main").path("compiler").default_condition(builder.config.compiler_docs) + run.crate_or_deps("rustc-main") + .path("compiler") + .default_condition(builder.config.compiler_docs) } fn make_run(run: RunConfig<'_>) { @@ -567,7 +569,7 @@ impl Step for Rustc { // Build the standard library, so that proc-macros can use it. // (Normally, only the metadata would be necessary, but proc-macros are special since they run at compile-time.) let compiler = builder.compiler(stage, builder.config.build); - builder.ensure(compile::Std { compiler, target: builder.config.build }); + builder.ensure(compile::Std::new(compiler, builder.config.build)); builder.info(&format!("Documenting stage{} compiler ({})", stage, target)); @@ -656,7 +658,7 @@ macro_rules! tool_doc { fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { let builder = run.builder; - run.krate($should_run).default_condition(builder.config.compiler_docs) + run.crate_or_deps($should_run).default_condition(builder.config.compiler_docs) } fn make_run(run: RunConfig<'_>) { @@ -683,7 +685,7 @@ macro_rules! tool_doc { // FIXME: is there a way to only ensure `check::Rustc` here? Last time I tried it failed // with strange errors, but only on a full bors test ... let compiler = builder.compiler(stage, builder.config.build); - builder.ensure(compile::Rustc { compiler, target }); + builder.ensure(compile::Rustc::new(compiler, target)); builder.info( &format!( @@ -866,7 +868,7 @@ impl Step for RustcBook { let rustc = builder.rustc(self.compiler); // The tool runs `rustc` for extracting output examples, so it needs a // functional sysroot. - builder.ensure(compile::Std { compiler: self.compiler, target: self.target }); + builder.ensure(compile::Std::new(self.compiler, self.target)); let mut cmd = builder.tool_cmd(Tool::LintDocs); cmd.arg("--src"); cmd.arg(builder.src.join("compiler")); diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 9958306b5765c..00550560ec22a 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -225,7 +225,7 @@ impl Step for Cargotest { /// test` to ensure that we don't regress the test suites there. fn run(self, builder: &Builder<'_>) { let compiler = builder.compiler(self.stage, self.host); - builder.ensure(compile::Rustc { compiler, target: compiler.host }); + builder.ensure(compile::Rustc::new(compiler, compiler.host)); let cargo = builder.ensure(tool::Cargo { compiler, target: compiler.host }); // Note that this is a short, cryptic, and not scoped directory name. This @@ -603,7 +603,7 @@ impl Step for CompiletestTest { // We need `ToolStd` for the locally-built sysroot because // compiletest uses unstable features of the `test` crate. - builder.ensure(compile::Std { compiler, target: host }); + builder.ensure(compile::Std::new(compiler, host)); let cargo = tool::prepare_tool_cargo( builder, compiler, @@ -896,7 +896,7 @@ impl Step for RustdocGUI { let nodejs = builder.config.nodejs.as_ref().expect("nodejs isn't available"); let npm = builder.config.npm.as_ref().expect("npm isn't available"); - builder.ensure(compile::Std { compiler: self.compiler, target: self.target }); + builder.ensure(compile::Std::new(self.compiler, self.target)); // The goal here is to check if the necessary packages are installed, and if not, we // panic. @@ -1273,12 +1273,12 @@ note: if you're sure you want to do this, please open an issue as to why. In the } if suite.ends_with("fulldeps") { - builder.ensure(compile::Rustc { compiler, target }); + builder.ensure(compile::Rustc::new(compiler, target)); } - builder.ensure(compile::Std { compiler, target }); + builder.ensure(compile::Std::new(compiler, target)); // ensure that `libproc_macro` is available on the host. - builder.ensure(compile::Std { compiler, target: compiler.host }); + builder.ensure(compile::Std::new(compiler, compiler.host)); // Also provide `rust_test_helpers` for the host. builder.ensure(native::TestHelpers { target: compiler.host }); @@ -1646,7 +1646,7 @@ impl BookTest { fn run_ext_doc(self, builder: &Builder<'_>) { let compiler = self.compiler; - builder.ensure(compile::Std { compiler, target: compiler.host }); + builder.ensure(compile::Std::new(compiler, compiler.host)); // mdbook just executes a binary named "rustdoc", so we need to update // PATH so that it points to our rustdoc. @@ -1674,7 +1674,7 @@ impl BookTest { fn run_local_doc(self, builder: &Builder<'_>) { let compiler = self.compiler; - builder.ensure(compile::Std { compiler, target: compiler.host }); + builder.ensure(compile::Std::new(compiler, compiler.host)); // Do a breadth-first traversal of the `src/doc` directory and just run // tests for all files that end in `*.md` @@ -1793,7 +1793,7 @@ impl Step for ErrorIndex { builder.run_quiet(&mut tool); // The tests themselves need to link to std, so make sure it is // available. - builder.ensure(compile::Std { compiler, target: compiler.host }); + builder.ensure(compile::Std::new(compiler, compiler.host)); markdown_test(builder, compiler, &output); } } @@ -1870,7 +1870,7 @@ impl Step for CrateLibrustc { const ONLY_HOSTS: bool = true; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.krate("rustc-main") + run.crate_or_deps("rustc-main") } fn make_run(run: RunConfig<'_>) { @@ -1912,7 +1912,7 @@ impl Step for Crate { const DEFAULT: bool = true; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.krate("test") + run.crate_or_deps("test") } fn make_run(run: RunConfig<'_>) { @@ -1943,7 +1943,7 @@ impl Step for Crate { let mode = self.mode; let test_kind = self.test_kind; - builder.ensure(compile::Std { compiler, target }); + builder.ensure(compile::Std::new(compiler, target)); builder.ensure(RemoteCopyLibs { compiler, target }); // If we're not doing a full bootstrap but we're testing a stage2 @@ -2065,7 +2065,7 @@ impl Step for CrateRustdoc { // isn't really necessary. builder.compiler_for(builder.top_stage, target, target) }; - builder.ensure(compile::Rustc { compiler, target }); + builder.ensure(compile::Rustc::new(compiler, target)); let mut cargo = tool::prepare_tool_cargo( builder, @@ -2180,7 +2180,7 @@ impl Step for CrateRustdocJsonTypes { // `compiler`, then it would cause rustdoc to be built *again*, which // isn't really necessary. let compiler = builder.compiler_for(builder.top_stage, target, target); - builder.ensure(compile::Rustc { compiler, target }); + builder.ensure(compile::Rustc::new(compiler, target)); let mut cargo = tool::prepare_tool_cargo( builder, @@ -2248,7 +2248,7 @@ impl Step for RemoteCopyLibs { return; } - builder.ensure(compile::Std { compiler, target }); + builder.ensure(compile::Std::new(compiler, target)); builder.info(&format!("REMOTE copy libs to emulator ({})", target)); @@ -2418,7 +2418,7 @@ impl Step for TierCheck { /// Tests the Platform Support page in the rustc book. fn run(self, builder: &Builder<'_>) { - builder.ensure(compile::Std { compiler: self.compiler, target: self.compiler.host }); + builder.ensure(compile::Std::new(self.compiler, self.compiler.host)); let mut cargo = tool::prepare_tool_cargo( builder, self.compiler, diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 905fa431d29d1..83e348ad9b755 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -51,10 +51,10 @@ impl Step for ToolBuild { match self.mode { Mode::ToolRustc => { - builder.ensure(compile::Std { compiler, target: compiler.host }); - builder.ensure(compile::Rustc { compiler, target }); + builder.ensure(compile::Std::new(compiler, compiler.host)); + builder.ensure(compile::Rustc::new(compiler, target)); } - Mode::ToolStd => builder.ensure(compile::Std { compiler, target }), + Mode::ToolStd => builder.ensure(compile::Std::new(compiler, target)), Mode::ToolBootstrap => {} // uses downloaded stage0 compiler libs _ => panic!("unexpected Mode for tool build"), } @@ -512,8 +512,8 @@ impl Step for Rustdoc { // When using `download-rustc` and a stage0 build_compiler, copying rustc doesn't actually // build stage0 libstd (because the libstd in sysroot has the wrong ABI). Explicitly build // it. - builder.ensure(compile::Std { compiler: build_compiler, target: target_compiler.host }); - builder.ensure(compile::Rustc { compiler: build_compiler, target: target_compiler.host }); + builder.ensure(compile::Std::new(build_compiler, target_compiler.host)); + builder.ensure(compile::Rustc::new(build_compiler, target_compiler.host)); // NOTE: this implies that `download-rustc` is pretty useless when compiling with the stage0 // compiler, since you do just as much work. if !builder.config.dry_run && builder.download_rustc() && build_compiler.stage == 0 { From 22b4ea4813a7aa5fc2ee454fca50853fdc47ba3e Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Sun, 3 Jul 2022 23:53:36 -0700 Subject: [PATCH 04/10] Proper macOS libLLVM symlink when cross compiling When cross compiling on macOS with `llvm.link-shared` enabled, the symlink creation will fail after compiling LLVM for the target architecture, because it will attempt to create the symlink in the host LLVM directory, which was already created when being built. This commit changes the symlink path to the actual LLVM output. --- src/bootstrap/native.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index 8395be40f9b52..503a2fc469e5d 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -487,16 +487,14 @@ impl Step for Llvm { let version = output(cmd.arg("--version")); let major = version.split('.').next().unwrap(); let lib_name = match llvm_version_suffix { - Some(s) => format!("lib/libLLVM-{}{}.dylib", major, s), - None => format!("lib/libLLVM-{}.dylib", major), + Some(s) => format!("libLLVM-{}{}.dylib", major, s), + None => format!("libLLVM-{}.dylib", major), }; - // The reason why we build the library path from llvm-config is because - // the output of llvm-config depends on its location in the file system. - // Make sure we create the symlink exactly where it's needed. - let llvm_base = build_llvm_config.parent().unwrap().parent().unwrap(); - let lib_llvm = llvm_base.join(lib_name); - t!(builder.symlink_file("libLLVM.dylib", &lib_llvm)); + let lib_llvm = out_dir.join("build").join("lib").join(lib_name); + if !lib_llvm.exists() { + t!(builder.symlink_file("libLLVM.dylib", &lib_llvm)); + } } t!(stamp.write()); From f8b16c5d8710efa065950adbdcd9238cd12a4505 Mon Sep 17 00:00:00 2001 From: pierwill Date: Tue, 5 Jul 2022 11:32:25 -0500 Subject: [PATCH 05/10] Edit `rustc_mir_dataflow::framework::lattice::FlatSet` docs Cosmetic improvements. Adds a paragraph break, and ellipses to signify arbitrary size of a flat set. --- compiler/rustc_mir_dataflow/src/framework/lattice.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/lattice.rs b/compiler/rustc_mir_dataflow/src/framework/lattice.rs index 9a462f6e1a41a..d6b89eb82275e 100644 --- a/compiler/rustc_mir_dataflow/src/framework/lattice.rs +++ b/compiler/rustc_mir_dataflow/src/framework/lattice.rs @@ -199,14 +199,16 @@ impl MeetSemiLattice for Dual { } /// Extends a type `T` with top and bottom elements to make it a partially ordered set in which no -/// value of `T` is comparable with any other. A flat set has the following [Hasse diagram]: +/// value of `T` is comparable with any other. +/// +/// A flat set has the following [Hasse diagram]: /// /// ```text -/// top -/// / / \ \ +/// top +/// / ... / / \ \ ... \ /// all possible values of `T` -/// \ \ / / -/// bottom +/// \ ... \ \ / / ... / +/// bottom /// ``` /// /// [Hasse diagram]: https://en.wikipedia.org/wiki/Hasse_diagram From cedc428a5fa988a3d7639a0fa45b726e0fd698ed Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 7 May 2022 15:01:25 +0200 Subject: [PATCH 06/10] fix the layout of repr(align) enums --- compiler/rustc_middle/src/ty/layout.rs | 6 +- src/test/ui/aligned_enum_cast.rs | 10 +- .../ui/layout/issue-96185-overaligned-enum.rs | 19 ++ .../issue-96185-overaligned-enum.stderr | 172 ++++++++++++++++++ 4 files changed, 203 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/layout/issue-96185-overaligned-enum.rs create mode 100644 src/test/ui/layout/issue-96185-overaligned-enum.stderr diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 3b05e42a53ead..e20f94b15c6ac 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -1418,9 +1418,9 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { if layout_variants.iter().all(|v| v.abi.is_uninhabited()) { abi = Abi::Uninhabited; - } else if tag.size(dl) == size || variants.iter().all(|layout| layout.is_empty()) { - // Without latter check aligned enums with custom discriminant values - // Would result in ICE see the issue #92464 for more info + } else if tag.size(dl) == size { + // Make sure we only use scalar layout when the enum is entirely its + // own tag (i.e. it has no padding nor any non-ZST variant fields). abi = Abi::Scalar(tag); } else { // Try to use a ScalarPair for all tagged enums. diff --git a/src/test/ui/aligned_enum_cast.rs b/src/test/ui/aligned_enum_cast.rs index 4b5776a6aa8ee..7fbfc760d09c8 100644 --- a/src/test/ui/aligned_enum_cast.rs +++ b/src/test/ui/aligned_enum_cast.rs @@ -11,5 +11,13 @@ enum Aligned { fn main() { let aligned = Aligned::Zero; let fo = aligned as u8; - println!("foo {}",fo); + println!("foo {}", fo); + println!("{}", tou8(Aligned::Zero)); +} + +#[inline(never)] +fn tou8(al: Aligned) -> u8 { + // Cast behind a function call so ConstProp does not see it + // (so that we can test codegen). + al as u8 } diff --git a/src/test/ui/layout/issue-96185-overaligned-enum.rs b/src/test/ui/layout/issue-96185-overaligned-enum.rs new file mode 100644 index 0000000000000..ae1e6b012c39b --- /dev/null +++ b/src/test/ui/layout/issue-96185-overaligned-enum.rs @@ -0,0 +1,19 @@ +// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN" +#![crate_type = "lib"] +#![feature(rustc_attrs)] + +// This cannot use `Scalar` abi since there is padding. +#[rustc_layout(debug)] +#[repr(align(8))] +pub enum Aligned1 { //~ ERROR: layout_of + Zero = 0, + One = 1, +} + +// This should use `Scalar` abi. +#[rustc_layout(debug)] +#[repr(align(1))] +pub enum Aligned2 { //~ ERROR: layout_of + Zero = 0, + One = 1, +} diff --git a/src/test/ui/layout/issue-96185-overaligned-enum.stderr b/src/test/ui/layout/issue-96185-overaligned-enum.stderr new file mode 100644 index 0000000000000..8dc364fa7c9bd --- /dev/null +++ b/src/test/ui/layout/issue-96185-overaligned-enum.stderr @@ -0,0 +1,172 @@ +error: layout_of(Aligned1) = Layout { + fields: Arbitrary { + offsets: [ + Size(0 bytes), + ], + memory_index: [ + 0, + ], + }, + variants: Multiple { + tag: Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + tag_encoding: Direct, + tag_field: 0, + variants: [ + Layout { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 0, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align(8 bytes), + pref: $PREF_ALIGN, + }, + size: Size(8 bytes), + }, + Layout { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 1, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align(8 bytes), + pref: $PREF_ALIGN, + }, + size: Size(8 bytes), + }, + ], + }, + abi: Aggregate { + sized: true, + }, + largest_niche: Some( + Niche { + offset: Size(0 bytes), + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + ), + align: AbiAndPrefAlign { + abi: Align(8 bytes), + pref: $PREF_ALIGN, + }, + size: Size(8 bytes), + } + --> $DIR/issue-96185-overaligned-enum.rs:8:1 + | +LL | pub enum Aligned1 { + | ^^^^^^^^^^^^^^^^^ + +error: layout_of(Aligned2) = Layout { + fields: Arbitrary { + offsets: [ + Size(0 bytes), + ], + memory_index: [ + 0, + ], + }, + variants: Multiple { + tag: Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + tag_encoding: Direct, + tag_field: 0, + variants: [ + Layout { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 0, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align(1 bytes), + pref: $PREF_ALIGN, + }, + size: Size(1 bytes), + }, + Layout { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 1, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align(1 bytes), + pref: $PREF_ALIGN, + }, + size: Size(1 bytes), + }, + ], + }, + abi: Scalar( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + ), + largest_niche: Some( + Niche { + offset: Size(0 bytes), + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + ), + align: AbiAndPrefAlign { + abi: Align(1 bytes), + pref: $PREF_ALIGN, + }, + size: Size(1 bytes), + } + --> $DIR/issue-96185-overaligned-enum.rs:16:1 + | +LL | pub enum Aligned2 { + | ^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + From d5721ce3a0f0d8eb2c46d87440b1977b5aef972c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Jul 2022 13:26:52 -0400 Subject: [PATCH 07/10] add asserts --- src/test/ui/aligned_enum_cast.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/ui/aligned_enum_cast.rs b/src/test/ui/aligned_enum_cast.rs index 7fbfc760d09c8..1ddf127172e65 100644 --- a/src/test/ui/aligned_enum_cast.rs +++ b/src/test/ui/aligned_enum_cast.rs @@ -12,7 +12,9 @@ fn main() { let aligned = Aligned::Zero; let fo = aligned as u8; println!("foo {}", fo); + assert_eq!(fo, 0); println!("{}", tou8(Aligned::Zero)); + assert_eq!(tou8(Aligned::Zero), 0); } #[inline(never)] From 15747dc91095ff1cbb3a335e0638a03cb2711885 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 5 Jul 2022 11:44:56 -0700 Subject: [PATCH 08/10] Update books --- src/doc/book | 2 +- src/doc/embedded-book | 2 +- src/doc/nomicon | 2 +- src/doc/rust-by-example | 2 +- src/doc/rustc-dev-guide | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/doc/book b/src/doc/book index efbafdba36184..cf2653a5ca553 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit efbafdba3618487fbc9305318fcab9775132ac15 +Subproject commit cf2653a5ca553cbbb4a17f1a7db1947820f6a775 diff --git a/src/doc/embedded-book b/src/doc/embedded-book index e17dcef5e9634..766979590da81 160000 --- a/src/doc/embedded-book +++ b/src/doc/embedded-book @@ -1 +1 @@ -Subproject commit e17dcef5e96346ee3d7fa56820ddc7e5c39636bc +Subproject commit 766979590da8100998f0d662499d4a901d8d1640 diff --git a/src/doc/nomicon b/src/doc/nomicon index 3a43983b76174..70db9e4189f64 160000 --- a/src/doc/nomicon +++ b/src/doc/nomicon @@ -1 +1 @@ -Subproject commit 3a43983b76174342b7dbd3e12ea2c49f762e52be +Subproject commit 70db9e4189f64d1d8e2451b1046111fb356b6dc2 diff --git a/src/doc/rust-by-example b/src/doc/rust-by-example index 1095df2a5850f..83724ca387a2a 160000 --- a/src/doc/rust-by-example +++ b/src/doc/rust-by-example @@ -1 +1 @@ -Subproject commit 1095df2a5850f2d345fad43a30633133365875ba +Subproject commit 83724ca387a2a1cd3e8d848f62820020760e358b diff --git a/src/doc/rustc-dev-guide b/src/doc/rustc-dev-guide index 048d925f0a955..eb83839e903a0 160000 --- a/src/doc/rustc-dev-guide +++ b/src/doc/rustc-dev-guide @@ -1 +1 @@ -Subproject commit 048d925f0a955aac601c4160c0e7f05771bcf63b +Subproject commit eb83839e903a0a8f1406f7e941886273f189b26b From c748551f7f006638d323cde079aaba30a49b185b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 19 Jun 2022 16:36:58 +0200 Subject: [PATCH 09/10] Fix invalid add of whitespace when there is where clause --- src/librustdoc/html/format.rs | 4 ++ src/librustdoc/html/render/print_item.rs | 55 ++++++++++++++++++------ 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 5584ecd287a53..0982c4b3acec8 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -146,6 +146,10 @@ impl Buffer { pub(crate) fn reserve(&mut self, additional: usize) { self.buffer.reserve(additional) } + + pub(crate) fn len(&self) -> usize { + self.buffer.len() + } } fn comma_sep( diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index fe00f952e043c..3525007baf369 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -62,6 +62,17 @@ struct ItemVars<'a> { src_href: Option<&'a str>, } +/// Calls `print_where_clause` and returns `true` if a `where` clause was generated. +fn print_where_clause_and_check<'a, 'tcx: 'a>( + buffer: &mut Buffer, + gens: &'a clean::Generics, + cx: &'a Context<'tcx>, +) -> bool { + let len_before = buffer.len(); + write!(buffer, "{}", print_where_clause(gens, cx, 0, true)); + len_before != buffer.len() +} + pub(super) fn print_item( cx: &mut Context<'_>, item: &clean::Item, @@ -1152,17 +1163,21 @@ fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean:: render_attributes_in_pre(w, it, ""); write!( w, - "{}enum {}{}{}", + "{}enum {}{}", it.visibility.print_with_space(it.item_id, cx), it.name.unwrap(), e.generics.print(cx), - print_where_clause(&e.generics, cx, 0, true), ); + if !print_where_clause_and_check(w, &e.generics, cx) { + // If there wasn't a `where` clause, we add a whitespace. + w.write_str(" "); + } + let variants_stripped = e.has_stripped_entries(); if count_variants == 0 && !variants_stripped { - w.write_str(" {}"); + w.write_str("{}"); } else { - w.write_str(" {\n"); + w.write_str("{\n"); let toggle = should_hide_fields(count_variants); if toggle { toggle_open(w, format_args!("{} variants", count_variants)); @@ -1643,13 +1658,21 @@ fn render_union( tab: &str, cx: &Context<'_>, ) { - write!(w, "{}union {}", it.visibility.print_with_space(it.item_id, cx), it.name.unwrap()); - if let Some(g) = g { - write!(w, "{}", g.print(cx)); - write!(w, "{}", print_where_clause(g, cx, 0, true)); + write!(w, "{}union {}", it.visibility.print_with_space(it.item_id, cx), it.name.unwrap(),); + + let where_displayed = g + .map(|g| { + write!(w, "{}", g.print(cx)); + print_where_clause_and_check(w, g, cx) + }) + .unwrap_or(false); + + // If there wasn't a `where` clause, we add a whitespace. + if !where_displayed { + w.write_str(" "); } - write!(w, " {{\n{}", tab); + write!(w, "{{\n{}", tab); let count_fields = fields.iter().filter(|f| matches!(*f.kind, clean::StructFieldItem(..))).count(); let toggle = should_hide_fields(count_fields); @@ -1701,10 +1724,14 @@ fn render_struct( } match ty { CtorKind::Fictive => { - if let Some(g) = g { - write!(w, "{}", print_where_clause(g, cx, 0, true),) + let where_diplayed = g.map(|g| print_where_clause_and_check(w, g, cx)).unwrap_or(false); + + // If there wasn't a `where` clause, we add a whitespace. + if !where_diplayed { + w.write_str(" {"); + } else { + w.write_str("{"); } - w.write_str(" {"); let count_fields = fields.iter().filter(|f| matches!(*f.kind, clean::StructFieldItem(..))).count(); let has_visible_fields = count_fields > 0; @@ -1759,7 +1786,7 @@ fn render_struct( } w.write_str(")"); if let Some(g) = g { - write!(w, "{}", print_where_clause(g, cx, 0, false),) + write!(w, "{}", print_where_clause(g, cx, 0, false)); } // We only want a ";" when we are displaying a tuple struct, not a variant tuple struct. if structhead { @@ -1769,7 +1796,7 @@ fn render_struct( CtorKind::Const => { // Needed for PhantomData. if let Some(g) = g { - write!(w, "{}", print_where_clause(g, cx, 0, false),) + write!(w, "{}", print_where_clause(g, cx, 0, false)); } w.write_str(";"); } From c8a5b671a0395706621430a57c258b85a30bad71 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 19 Jun 2022 16:38:10 +0200 Subject: [PATCH 10/10] Add test for invalid whitespace display after where clause --- .../whitespace-after-where-clause.enum.html | 4 + .../whitespace-after-where-clause.enum2.html | 4 + .../rustdoc/whitespace-after-where-clause.rs | 77 +++++++++++++++++++ .../whitespace-after-where-clause.struct.html | 4 + ...whitespace-after-where-clause.struct2.html | 4 + .../whitespace-after-where-clause.trait.html | 6 ++ .../whitespace-after-where-clause.trait2.html | 6 ++ .../whitespace-after-where-clause.union.html | 3 + .../whitespace-after-where-clause.union2.html | 3 + 9 files changed, 111 insertions(+) create mode 100644 src/test/rustdoc/whitespace-after-where-clause.enum.html create mode 100644 src/test/rustdoc/whitespace-after-where-clause.enum2.html create mode 100644 src/test/rustdoc/whitespace-after-where-clause.rs create mode 100644 src/test/rustdoc/whitespace-after-where-clause.struct.html create mode 100644 src/test/rustdoc/whitespace-after-where-clause.struct2.html create mode 100644 src/test/rustdoc/whitespace-after-where-clause.trait.html create mode 100644 src/test/rustdoc/whitespace-after-where-clause.trait2.html create mode 100644 src/test/rustdoc/whitespace-after-where-clause.union.html create mode 100644 src/test/rustdoc/whitespace-after-where-clause.union2.html diff --git a/src/test/rustdoc/whitespace-after-where-clause.enum.html b/src/test/rustdoc/whitespace-after-where-clause.enum.html new file mode 100644 index 0000000000000..9e5bf45ae7d2f --- /dev/null +++ b/src/test/rustdoc/whitespace-after-where-clause.enum.html @@ -0,0 +1,4 @@ +
pub enum Cow<'a, B: ?Sized + 'a> where
    B: ToOwned<dyn Clone>, {
+    Borrowed(&'a B),
+    Whatever(u32),
+}
diff --git a/src/test/rustdoc/whitespace-after-where-clause.enum2.html b/src/test/rustdoc/whitespace-after-where-clause.enum2.html new file mode 100644 index 0000000000000..6bc47beaed125 --- /dev/null +++ b/src/test/rustdoc/whitespace-after-where-clause.enum2.html @@ -0,0 +1,4 @@ +
pub enum Cow2<'a, B: ?Sized + ToOwned<dyn Clone> + 'a> {
+    Borrowed(&'a B),
+    Whatever(u32),
+}
diff --git a/src/test/rustdoc/whitespace-after-where-clause.rs b/src/test/rustdoc/whitespace-after-where-clause.rs new file mode 100644 index 0000000000000..c36386a2aa2b5 --- /dev/null +++ b/src/test/rustdoc/whitespace-after-where-clause.rs @@ -0,0 +1,77 @@ +// This test ensures there is no whitespace before the first brace of +// trait, enum, struct and union items when they have a where clause. + +#![crate_name = "foo"] + +// @has 'foo/trait.ToOwned.html' +// @snapshot trait - '//*[@class="docblock item-decl"]' +pub trait ToOwned +where T: Clone +{ + type Owned; + fn to_owned(&self) -> Self::Owned; + fn whatever(&self) -> T; +} + +// @has 'foo/trait.ToOwned2.html' +// @snapshot trait2 - '//*[@class="docblock item-decl"]' +// There should be a whitespace before `{` in this case! +pub trait ToOwned2 { + type Owned; + fn to_owned(&self) -> Self::Owned; + fn whatever(&self) -> T; +} + +// @has 'foo/enum.Cow.html' +// @snapshot enum - '//*[@class="docblock item-decl"]' +pub enum Cow<'a, B: ?Sized + 'a> +where + B: ToOwned, +{ + Borrowed(&'a B), + Whatever(u32), +} + +// @has 'foo/enum.Cow2.html' +// @snapshot enum2 - '//*[@class="docblock item-decl"]' +// There should be a whitespace before `{` in this case! +pub enum Cow2<'a, B: ?Sized + ToOwned + 'a> { + Borrowed(&'a B), + Whatever(u32), +} + +// @has 'foo/struct.Struct.html' +// @snapshot struct - '//*[@class="docblock item-decl"]' +pub struct Struct<'a, B: ?Sized + 'a> +where + B: ToOwned, +{ + pub a: &'a B, + pub b: u32, +} + +// @has 'foo/struct.Struct2.html' +// @snapshot struct2 - '//*[@class="docblock item-decl"]' +// There should be a whitespace before `{` in this case! +pub struct Struct2<'a, B: ?Sized + ToOwned + 'a> { + pub a: &'a B, + pub b: u32, +} + +// @has 'foo/union.Union.html' +// @snapshot union - '//*[@class="docblock item-decl"]' +pub union Union<'a, B: ?Sized + 'a> +where + B: ToOwned, +{ + a: &'a B, + b: u32, +} + +// @has 'foo/union.Union2.html' +// @snapshot union2 - '//*[@class="docblock item-decl"]' +// There should be a whitespace before `{` in this case! +pub union Union2<'a, B: ?Sized + ToOwned + 'a> { + a: &'a B, + b: u32, +} diff --git a/src/test/rustdoc/whitespace-after-where-clause.struct.html b/src/test/rustdoc/whitespace-after-where-clause.struct.html new file mode 100644 index 0000000000000..236cc3b30d088 --- /dev/null +++ b/src/test/rustdoc/whitespace-after-where-clause.struct.html @@ -0,0 +1,4 @@ +
pub struct Struct<'a, B: ?Sized + 'a> where
    B: ToOwned<dyn Clone>, {
+    pub a: &'a B,
+    pub b: u32,
+}
diff --git a/src/test/rustdoc/whitespace-after-where-clause.struct2.html b/src/test/rustdoc/whitespace-after-where-clause.struct2.html new file mode 100644 index 0000000000000..47f5c6ba9c846 --- /dev/null +++ b/src/test/rustdoc/whitespace-after-where-clause.struct2.html @@ -0,0 +1,4 @@ +
pub struct Struct2<'a, B: ?Sized + ToOwned<dyn Clone> + 'a> {
+    pub a: &'a B,
+    pub b: u32,
+}
diff --git a/src/test/rustdoc/whitespace-after-where-clause.trait.html b/src/test/rustdoc/whitespace-after-where-clause.trait.html new file mode 100644 index 0000000000000..98f03b837b528 --- /dev/null +++ b/src/test/rustdoc/whitespace-after-where-clause.trait.html @@ -0,0 +1,6 @@ +
pub trait ToOwned<T> where
    T: Clone, {
+    type Owned;
+
+    fn to_owned(&self) -> Self::Owned;
+    fn whatever(&self) -> T;
+}
diff --git a/src/test/rustdoc/whitespace-after-where-clause.trait2.html b/src/test/rustdoc/whitespace-after-where-clause.trait2.html new file mode 100644 index 0000000000000..35052869e762d --- /dev/null +++ b/src/test/rustdoc/whitespace-after-where-clause.trait2.html @@ -0,0 +1,6 @@ +
pub trait ToOwned2<T: Clone> {
+    type Owned;
+
+    fn to_owned(&self) -> Self::Owned;
+    fn whatever(&self) -> T;
+}
diff --git a/src/test/rustdoc/whitespace-after-where-clause.union.html b/src/test/rustdoc/whitespace-after-where-clause.union.html new file mode 100644 index 0000000000000..97e1bbcf339f8 --- /dev/null +++ b/src/test/rustdoc/whitespace-after-where-clause.union.html @@ -0,0 +1,3 @@ +
pub union Union<'a, B: ?Sized + 'a> where
    B: ToOwned<dyn Clone>, {
+    /* private fields */
+}
diff --git a/src/test/rustdoc/whitespace-after-where-clause.union2.html b/src/test/rustdoc/whitespace-after-where-clause.union2.html new file mode 100644 index 0000000000000..6c752a8b4c5ea --- /dev/null +++ b/src/test/rustdoc/whitespace-after-where-clause.union2.html @@ -0,0 +1,3 @@ +
pub union Union2<'a, B: ?Sized + ToOwned<dyn Clone> + 'a> {
+    /* private fields */
+}