From ab683901531a61a19a459b981d262372124756b2 Mon Sep 17 00:00:00 2001 From: Zachary Whiteley Date: Sat, 10 Jun 2023 09:06:51 +0100 Subject: [PATCH 1/5] Improve documentation for `tools` profile Make the build process more beginner friendly: - Include information explaining that the stage2 toolchain should be used (and not the stage1 toolchain) due to the `download-rustc` setting. - Display a message when the user runs `x setup tools` explaining that they should use the stage2 toolchain. --- src/bootstrap/defaults/config.tools.toml | 2 ++ src/bootstrap/setup.rs | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/src/bootstrap/defaults/config.tools.toml b/src/bootstrap/defaults/config.tools.toml index 6b6625342a67e..79424f28d27b7 100644 --- a/src/bootstrap/defaults/config.tools.toml +++ b/src/bootstrap/defaults/config.tools.toml @@ -9,6 +9,8 @@ debug-logging = true incremental = true # Download rustc from CI instead of building it from source. # This cuts compile times by almost 60x, but means you can't modify the compiler. +# Using these defaults will download the stage2 compiler (see `download-rustc` +# setting) and the stage2 toolchain should therefore be used for these defaults. download-rustc = "if-unchanged" [build] diff --git a/src/bootstrap/setup.rs b/src/bootstrap/setup.rs index c604c63a4dd76..e13f3f0bd5eb7 100644 --- a/src/bootstrap/setup.rs +++ b/src/bootstrap/setup.rs @@ -176,6 +176,14 @@ pub fn setup(config: &Config, profile: Profile) { ); } + if profile == Profile::Tools { + eprintln!(); + eprintln!( + "note: the `tools` profile sets up the `stage2` toolchain (use \ + `rustup toolchain link 'name' host/build/stage2` to use rustc)" + ) + } + let path = &config.config.clone().unwrap_or(PathBuf::from("config.toml")); setup_config_toml(path, profile, config); } From e49b4625df65bdbbd36de83a3f209a8031d65fec Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 10 Jun 2023 23:17:02 +0000 Subject: [PATCH 2/5] Don't compute opt_suggest_box_span for TAIT --- compiler/rustc_hir_typeck/src/_match.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index 7d2f7e876083a..444ff90595c23 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -510,6 +510,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .. } = self.type_var_origin(expected)? else { return None; }; + let Some(rpit_local_def_id) = rpit_def_id.as_local() else { return None; }; + if !matches!( + self.tcx.hir().expect_item(rpit_local_def_id).expect_opaque_ty().origin, + hir::OpaqueTyOrigin::FnReturn(..) + ) { + return None; + } + let sig = self.body_fn_sig()?; let substs = sig.output().walk().find_map(|arg| { From 5dfc17f045aca52ad37a768d0f6704ea5b4fd4dd Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 10 Jun 2023 23:36:55 +0000 Subject: [PATCH 3/5] prune some unused code --- compiler/rustc_hir_typeck/src/coercion.rs | 113 +--------------------- 1 file changed, 4 insertions(+), 109 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 81231e8fe068b..79157eae7ed6d 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -36,9 +36,7 @@ //! ``` use crate::FnCtxt; -use rustc_errors::{ - struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan, -}; +use rustc_errors::{struct_span_err, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, Visitor}; @@ -58,7 +56,7 @@ use rustc_middle::ty::visit::TypeVisitableExt; use rustc_middle::ty::{self, Ty, TypeAndMut}; use rustc_session::parse::feature_err; use rustc_span::symbol::sym; -use rustc_span::{self, BytePos, DesugaringKind, Span}; +use rustc_span::{self, DesugaringKind}; use rustc_target::spec::abi::Abi; use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _; @@ -1702,9 +1700,6 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { ) -> DiagnosticBuilder<'a, ErrorGuaranteed> { let mut err = fcx.err_ctxt().report_mismatched_types(cause, expected, found, ty_err); - let mut pointing_at_return_type = false; - let mut fn_output = None; - let parent_id = fcx.tcx.hir().parent_id(id); let parent = fcx.tcx.hir().get(parent_id); if let Some(expr) = expression @@ -1717,7 +1712,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { // label pointing out the cause for the type coercion will be wrong // as prior return coercions would not be relevant (#57664). let fn_decl = if let (Some(expr), Some(blk_id)) = (expression, blk_id) { - pointing_at_return_type = + let pointing_at_return_type = fcx.suggest_mismatched_types_on_tail(&mut err, expr, expected, found, blk_id); if let (Some(cond_expr), true, false) = ( fcx.tcx.hir().get_if_cause(expr.hir_id), @@ -1749,7 +1744,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { if let Some((fn_id, fn_decl, can_suggest)) = fn_decl { if blk_id.is_none() { - pointing_at_return_type |= fcx.suggest_missing_return_type( + fcx.suggest_missing_return_type( &mut err, &fn_decl, expected, @@ -1758,9 +1753,6 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { fn_id, ); } - if !pointing_at_return_type { - fn_output = Some(&fn_decl.output); // `impl Trait` return type - } } let parent_id = fcx.tcx.hir().get_parent_item(id); @@ -1795,106 +1787,9 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { ); } - if let (Some(sp), Some(fn_output)) = (ret_coercion_span, fn_output) { - self.add_impl_trait_explanation(&mut err, cause, fcx, expected, sp, fn_output); - } - err } - fn add_impl_trait_explanation<'a>( - &self, - err: &mut Diagnostic, - cause: &ObligationCause<'tcx>, - fcx: &FnCtxt<'a, 'tcx>, - expected: Ty<'tcx>, - sp: Span, - fn_output: &hir::FnRetTy<'_>, - ) { - let return_sp = fn_output.span(); - err.span_label(return_sp, "expected because this return type..."); - err.span_label( - sp, - format!("...is found to be `{}` here", fcx.resolve_vars_with_obligations(expected)), - ); - let impl_trait_msg = "for information on `impl Trait`, see \ - "; - let trait_obj_msg = "for information on trait objects, see \ - "; - err.note("to return `impl Trait`, all returned values must be of the same type"); - err.note(impl_trait_msg); - let snippet = fcx - .tcx - .sess - .source_map() - .span_to_snippet(return_sp) - .unwrap_or_else(|_| "dyn Trait".to_string()); - let mut snippet_iter = snippet.split_whitespace(); - let has_impl = snippet_iter.next().is_some_and(|s| s == "impl"); - // Only suggest `Box` if `Trait` in `impl Trait` is object safe. - let mut is_object_safe = false; - if let hir::FnRetTy::Return(ty) = fn_output - // Get the return type. - && let hir::TyKind::OpaqueDef(..) = ty.kind - { - let ty = fcx.astconv().ast_ty_to_ty( ty); - // Get the `impl Trait`'s `DefId`. - if let ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }) = ty.kind() - // Get the `impl Trait`'s `Item` so that we can get its trait bounds and - // get the `Trait`'s `DefId`. - && let hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds, .. }) = - fcx.tcx.hir().expect_item(def_id.expect_local()).kind - { - // Are of this `impl Trait`'s traits object safe? - is_object_safe = bounds.iter().all(|bound| { - bound - .trait_ref() - .and_then(|t| t.trait_def_id()) - .is_some_and(|def_id| { - fcx.tcx.check_is_object_safe(def_id) - }) - }) - } - }; - if has_impl { - if is_object_safe { - err.multipart_suggestion( - "you could change the return type to be a boxed trait object", - vec![ - (return_sp.with_hi(return_sp.lo() + BytePos(4)), "Box".to_string()), - ], - Applicability::MachineApplicable, - ); - let sugg = [sp, cause.span] - .into_iter() - .flat_map(|sp| { - [ - (sp.shrink_to_lo(), "Box::new(".to_string()), - (sp.shrink_to_hi(), ")".to_string()), - ] - .into_iter() - }) - .collect::>(); - err.multipart_suggestion( - "if you change the return type to expect trait objects, box the returned \ - expressions", - sugg, - Applicability::MaybeIncorrect, - ); - } else { - err.help(format!( - "if the trait `{}` were object safe, you could return a boxed trait object", - &snippet[5..] - )); - } - err.note(trait_obj_msg); - } - err.help("you could instead create a new `enum` with a variant for each returned type"); - } - /// Checks whether the return type is unsized via an obligation, which makes /// sure we consider `dyn Trait: Sized` where clauses, which are trivially /// false but technically valid for typeck. From d80440263c3eb083a91335ab14bee76bcb9988d1 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 11 Jun 2023 00:19:56 +0000 Subject: [PATCH 4/5] Don't suggest boxing an empty if/else arm --- .../src/infer/error_reporting/mod.rs | 20 ++++++++++++++++++- .../dont-suggest-box-on-empty-else-arm.rs | 9 +++++++++ .../dont-suggest-box-on-empty-else-arm.stderr | 16 +++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.rs create mode 100644 tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.stderr diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index ce5b5882e3b26..3e4812e7ca995 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -847,7 +847,25 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ) { err.subdiagnostic(subdiag); } - if let Some(ret_sp) = opt_suggest_box_span { + // don't suggest wrapping either blocks in `if .. {} else {}` + let is_empty_arm = |id| { + let hir::Node::Block(blk) = self.tcx.hir().get(id) + else { + return false; + }; + if blk.expr.is_some() || !blk.stmts.is_empty() { + return false; + } + let Some((_, hir::Node::Expr(expr))) = self.tcx.hir().parent_iter(id).nth(1) + else { + return false; + }; + matches!(expr.kind, hir::ExprKind::If(..)) + }; + if let Some(ret_sp) = opt_suggest_box_span + && !is_empty_arm(then_id) + && !is_empty_arm(else_id) + { self.suggest_boxing_for_return_impl_trait( err, ret_sp, diff --git a/tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.rs b/tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.rs new file mode 100644 index 0000000000000..befd768b1818a --- /dev/null +++ b/tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.rs @@ -0,0 +1,9 @@ +fn test() -> impl std::fmt::Debug { + if true { + "boo2" + } else { + //~^ ERROR `if` and `else` have incompatible types + } +} + +fn main() {} diff --git a/tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.stderr b/tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.stderr new file mode 100644 index 0000000000000..9b63911da7aa3 --- /dev/null +++ b/tests/ui/impl-trait/dont-suggest-box-on-empty-else-arm.stderr @@ -0,0 +1,16 @@ +error[E0308]: `if` and `else` have incompatible types + --> $DIR/dont-suggest-box-on-empty-else-arm.rs:4:12 + | +LL | if true { + | ------- `if` and `else` have incompatible types +LL | "boo2" + | ------ expected because of this +LL | } else { + | ____________^ +LL | | +LL | | } + | |_____^ expected `&str`, found `()` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 123693953f6fb4063f95bd59b288aea8375c3c34 Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 11 Jun 2023 10:13:02 -0500 Subject: [PATCH 5/5] Don't override `debuginfo-level = 1` to mean `line-tables-only` This has real differences in the effective debuginfo: in particular, it omits the module-level information and breaks perf. Allow passing `line-tables-only` directly in config.toml instead. --- src/bootstrap/builder.rs | 7 +--- src/bootstrap/config.rs | 86 +++++++++++++++++++++++++++++++++------- 2 files changed, 73 insertions(+), 20 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index ea1b34812e5ca..51d94c48f7ffb 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -1649,12 +1649,7 @@ impl<'a> Builder<'a> { self.config.rust_debuginfo_level_tools } }; - if debuginfo_level == 1 { - // Use less debuginfo than the default to save on disk space. - cargo.env(profile_var("DEBUG"), "line-tables-only"); - } else { - cargo.env(profile_var("DEBUG"), debuginfo_level.to_string()); - }; + cargo.env(profile_var("DEBUG"), debuginfo_level.to_string()); if self.cc[&target].args().iter().any(|arg| arg == "-gz") { rustflags.arg("-Clink-arg=-gz"); } diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index c59df7eecf680..b521ad75d63a1 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -10,7 +10,7 @@ use std::cell::{Cell, RefCell}; use std::cmp; use std::collections::{HashMap, HashSet}; use std::env; -use std::fmt; +use std::fmt::{self, Display}; use std::fs; use std::io::IsTerminal; use std::path::{Path, PathBuf}; @@ -50,6 +50,57 @@ pub enum DryRun { UserSelected, } +#[derive(Copy, Clone, Default)] +pub enum DebuginfoLevel { + #[default] + None, + LineTablesOnly, + Limited, + Full, +} + +// NOTE: can't derive(Deserialize) because the intermediate trip through toml::Value only +// deserializes i64, and derive() only generates visit_u64 +impl<'de> Deserialize<'de> for DebuginfoLevel { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + use serde::de::Error; + + Ok(match Deserialize::deserialize(deserializer)? { + StringOrInt::String("none") | StringOrInt::Int(0) => DebuginfoLevel::None, + StringOrInt::String("line-tables-only") => DebuginfoLevel::LineTablesOnly, + StringOrInt::String("limited") | StringOrInt::Int(1) => DebuginfoLevel::Limited, + StringOrInt::String("full") | StringOrInt::Int(2) => DebuginfoLevel::Full, + StringOrInt::Int(n) => { + let other = serde::de::Unexpected::Signed(n); + return Err(D::Error::invalid_value(other, &"expected 0, 1, or 2")); + } + StringOrInt::String(s) => { + let other = serde::de::Unexpected::Str(s); + return Err(D::Error::invalid_value( + other, + &"expected none, line-tables-only, limited, or full", + )); + } + }) + } +} + +/// Suitable for passing to `-C debuginfo` +impl Display for DebuginfoLevel { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use DebuginfoLevel::*; + f.write_str(match self { + None => "0", + LineTablesOnly => "line-tables-only", + Limited => "1", + Full => "2", + }) + } +} + /// Global configuration for the entire build and/or bootstrap. /// /// This structure is parsed from `config.toml`, and some of the fields are inferred from `git` or build-time parameters. @@ -159,10 +210,10 @@ pub struct Config { pub rust_overflow_checks: bool, pub rust_overflow_checks_std: bool, pub rust_debug_logging: bool, - pub rust_debuginfo_level_rustc: u32, - pub rust_debuginfo_level_std: u32, - pub rust_debuginfo_level_tools: u32, - pub rust_debuginfo_level_tests: u32, + pub rust_debuginfo_level_rustc: DebuginfoLevel, + pub rust_debuginfo_level_std: DebuginfoLevel, + pub rust_debuginfo_level_tools: DebuginfoLevel, + pub rust_debuginfo_level_tests: DebuginfoLevel, pub rust_split_debuginfo: SplitDebuginfo, pub rust_rpath: bool, pub rustc_parallel: bool, @@ -810,6 +861,13 @@ impl Default for StringOrBool { } } +#[derive(Deserialize)] +#[serde(untagged)] +enum StringOrInt<'a> { + String(&'a str), + Int(i64), +} + define_config! { /// TOML representation of how the Rust build is configured. struct Rust { @@ -822,11 +880,11 @@ define_config! { overflow_checks: Option = "overflow-checks", overflow_checks_std: Option = "overflow-checks-std", debug_logging: Option = "debug-logging", - debuginfo_level: Option = "debuginfo-level", - debuginfo_level_rustc: Option = "debuginfo-level-rustc", - debuginfo_level_std: Option = "debuginfo-level-std", - debuginfo_level_tools: Option = "debuginfo-level-tools", - debuginfo_level_tests: Option = "debuginfo-level-tests", + debuginfo_level: Option = "debuginfo-level", + debuginfo_level_rustc: Option = "debuginfo-level-rustc", + debuginfo_level_std: Option = "debuginfo-level-std", + debuginfo_level_tools: Option = "debuginfo-level-tools", + debuginfo_level_tests: Option = "debuginfo-level-tests", split_debuginfo: Option = "split-debuginfo", run_dsymutil: Option = "run-dsymutil", backtrace: Option = "backtrace", @@ -1478,17 +1536,17 @@ impl Config { config.rust_debug_logging = debug_logging.unwrap_or(config.rust_debug_assertions); - let with_defaults = |debuginfo_level_specific: Option| { + let with_defaults = |debuginfo_level_specific: Option<_>| { debuginfo_level_specific.or(debuginfo_level).unwrap_or(if debug == Some(true) { - 1 + DebuginfoLevel::Limited } else { - 0 + DebuginfoLevel::None }) }; config.rust_debuginfo_level_rustc = with_defaults(debuginfo_level_rustc); config.rust_debuginfo_level_std = with_defaults(debuginfo_level_std); config.rust_debuginfo_level_tools = with_defaults(debuginfo_level_tools); - config.rust_debuginfo_level_tests = debuginfo_level_tests.unwrap_or(0); + config.rust_debuginfo_level_tests = debuginfo_level_tests.unwrap_or(DebuginfoLevel::None); let download_rustc = config.download_rustc_commit.is_some(); // See https://github.com/rust-lang/compiler-team/issues/326