From 3fe0b3d4d04dd86cbe22770217f3b42a8856d382 Mon Sep 17 00:00:00 2001 From: Mu001999 Date: Sun, 23 Feb 2025 11:29:35 +0800 Subject: [PATCH 01/67] not lint break with label and unsafe block --- compiler/rustc_parse/src/parser/expr.rs | 14 ++++++++------ tests/ui/lint/break-with-label-and-unsafe-block.rs | 11 +++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 tests/ui/lint/break-with-label-and-unsafe-block.rs diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index e0e6c2177da..21e01ff535a 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1859,13 +1859,15 @@ impl<'a> Parser<'a> { let mut expr = self.parse_expr_opt()?; if let Some(expr) = &mut expr { if label.is_some() - && matches!( - expr.kind, + && match &expr.kind { ExprKind::While(_, _, None) - | ExprKind::ForLoop { label: None, .. } - | ExprKind::Loop(_, None, _) - | ExprKind::Block(_, None) - ) + | ExprKind::ForLoop { label: None, .. } + | ExprKind::Loop(_, None, _) => true, + ExprKind::Block(block, None) => { + matches!(block.rules, BlockCheckMode::Default) + } + _ => false, + } { self.psess.buffer_lint( BREAK_WITH_LABEL_AND_LOOP, diff --git a/tests/ui/lint/break-with-label-and-unsafe-block.rs b/tests/ui/lint/break-with-label-and-unsafe-block.rs new file mode 100644 index 00000000000..a76a5761475 --- /dev/null +++ b/tests/ui/lint/break-with-label-and-unsafe-block.rs @@ -0,0 +1,11 @@ +//@ check-pass + +#![deny(break_with_label_and_loop)] + +unsafe fn foo() -> i32 { 42 } + +fn main () { + 'label: loop { + break 'label unsafe { foo() } + }; +} From 049bb266873ff98ce8219c7d2d5d56c7cbe3eaf3 Mon Sep 17 00:00:00 2001 From: beetrees Date: Sun, 23 Mar 2025 22:50:28 +0000 Subject: [PATCH 02/67] Add target-specific NaN payloads for the missing tier 2 targets --- library/core/src/primitive_docs.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/core/src/primitive_docs.rs b/library/core/src/primitive_docs.rs index 89c856fe107..6c59e71194b 100644 --- a/library/core/src/primitive_docs.rs +++ b/library/core/src/primitive_docs.rs @@ -1309,10 +1309,12 @@ mod prim_f16 {} // FIXME: Is there a better place to put this? /// /// | `target_arch` | Extra payloads possible on this platform | -/// |---------------|---------| -/// | `x86`, `x86_64`, `arm`, `aarch64`, `riscv32`, `riscv64` | None | +/// |---------------|------------------------------------------| +// Sorted alphabetically +/// | `aarch64`, `arm`, `arm64ec`, `loongarch64`, `powerpc` (except when `target_abi = "spe"`), `powerpc64`, `riscv32`, `riscv64`, `s390x`, `x86`, `x86_64` | None | +/// | `nvptx64` | All payloads | /// | `sparc`, `sparc64` | The all-one payload | -/// | `wasm32`, `wasm64` | If all input NaNs are quiet with all-zero payload: None.
Otherwise: all possible payloads. | +/// | `wasm32`, `wasm64` | If all input NaNs are quiet with all-zero payload: None.
Otherwise: all payloads. | /// /// For targets not in this table, all payloads are possible. From 3ee62a906e8a025834d11c46b8c2f133d3a12448 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 30 Mar 2025 18:52:25 +0000 Subject: [PATCH 03/67] Do not optimize out SwitchInt before borrowck, or if Zmir-preserve-ub --- compiler/rustc_interface/src/tests.rs | 2 +- .../src/early_otherwise_branch.rs | 2 +- compiler/rustc_mir_transform/src/inline.rs | 4 +- .../rustc_mir_transform/src/match_branches.rs | 2 +- .../src/remove_place_mention.rs | 2 +- .../src/remove_unneeded_drops.rs | 2 +- compiler/rustc_mir_transform/src/simplify.rs | 35 +++++++++---- compiler/rustc_session/src/options.rs | 6 +-- src/tools/miri/src/lib.rs | 2 +- .../tests/fail/read_from_trivial_switch.rs | 14 ++++++ .../fail/read_from_trivial_switch.stderr | 15 ++++++ ....match_tuple.SimplifyCfg-initial.after.mir | 34 +++++++------ .../dead-store-elimination/place_mention.rs | 2 +- ...le_switchint.SimplifyCfg-initial.after.mir | 38 ++++++++------ ...ivial_switch.main.SimplifyCfg-initial.diff | 49 +++++++++++++++++++ tests/mir-opt/read_from_trivial_switch.rs | 15 ++++++ tests/ui/pattern/uninit-trivial.rs | 8 +++ tests/ui/pattern/uninit-trivial.stderr | 16 ++++++ 18 files changed, 197 insertions(+), 51 deletions(-) create mode 100644 src/tools/miri/tests/fail/read_from_trivial_switch.rs create mode 100644 src/tools/miri/tests/fail/read_from_trivial_switch.stderr create mode 100644 tests/mir-opt/read_from_trivial_switch.main.SimplifyCfg-initial.diff create mode 100644 tests/mir-opt/read_from_trivial_switch.rs create mode 100644 tests/ui/pattern/uninit-trivial.rs create mode 100644 tests/ui/pattern/uninit-trivial.stderr diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index a8e55663257..af30a2d8aa8 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -817,8 +817,8 @@ fn test_unstable_options_tracking_hash() { tracked!(min_function_alignment, Some(Align::EIGHT)); tracked!(mir_emit_retag, true); tracked!(mir_enable_passes, vec![("DestProp".to_string(), false)]); - tracked!(mir_keep_place_mention, true); tracked!(mir_opt_level, Some(4)); + tracked!(mir_preserve_ub, true); tracked!(move_size_limit, Some(4096)); tracked!(mutable_noalias, false); tracked!(next_solver, NextSolverConfig { coherence: true, globally: true }); diff --git a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs index 57f7893be1b..ac97b08fbed 100644 --- a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs +++ b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs @@ -224,7 +224,7 @@ impl<'tcx> crate::MirPass<'tcx> for EarlyOtherwiseBranch { // Since this optimization adds new basic blocks and invalidates others, // clean up the cfg to make it nicer for other passes if should_cleanup { - simplify_cfg(body); + simplify_cfg(tcx, body); } } diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 0ab24e48d44..babee81a2ad 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -63,7 +63,7 @@ impl<'tcx> crate::MirPass<'tcx> for Inline { let _guard = span.enter(); if inline::>(tcx, body) { debug!("running simplify cfg on {:?}", body.source); - simplify_cfg(body); + simplify_cfg(tcx, body); deref_finder(tcx, body); } } @@ -99,7 +99,7 @@ impl<'tcx> crate::MirPass<'tcx> for ForceInline { let _guard = span.enter(); if inline::>(tcx, body) { debug!("running simplify cfg on {:?}", body.source); - simplify_cfg(body); + simplify_cfg(tcx, body); deref_finder(tcx, body); } } diff --git a/compiler/rustc_mir_transform/src/match_branches.rs b/compiler/rustc_mir_transform/src/match_branches.rs index 0d9d0368d37..d43a35124c2 100644 --- a/compiler/rustc_mir_transform/src/match_branches.rs +++ b/compiler/rustc_mir_transform/src/match_branches.rs @@ -45,7 +45,7 @@ impl<'tcx> crate::MirPass<'tcx> for MatchBranchSimplification { } if should_cleanup { - simplify_cfg(body); + simplify_cfg(tcx, body); } } diff --git a/compiler/rustc_mir_transform/src/remove_place_mention.rs b/compiler/rustc_mir_transform/src/remove_place_mention.rs index 15fe77d5319..cb598ceb4df 100644 --- a/compiler/rustc_mir_transform/src/remove_place_mention.rs +++ b/compiler/rustc_mir_transform/src/remove_place_mention.rs @@ -8,7 +8,7 @@ pub(super) struct RemovePlaceMention; impl<'tcx> crate::MirPass<'tcx> for RemovePlaceMention { fn is_enabled(&self, sess: &rustc_session::Session) -> bool { - !sess.opts.unstable_opts.mir_keep_place_mention + !sess.opts.unstable_opts.mir_preserve_ub } fn run_pass(&self, _: TyCtxt<'tcx>, body: &mut Body<'tcx>) { diff --git a/compiler/rustc_mir_transform/src/remove_unneeded_drops.rs b/compiler/rustc_mir_transform/src/remove_unneeded_drops.rs index 8a8cdafc690..43f80508e4a 100644 --- a/compiler/rustc_mir_transform/src/remove_unneeded_drops.rs +++ b/compiler/rustc_mir_transform/src/remove_unneeded_drops.rs @@ -35,7 +35,7 @@ impl<'tcx> crate::MirPass<'tcx> for RemoveUnneededDrops { // if we applied optimizations, we potentially have some cfg to cleanup to // make it easier for further passes if should_simplify { - simplify_cfg(body); + simplify_cfg(tcx, body); } } diff --git a/compiler/rustc_mir_transform/src/simplify.rs b/compiler/rustc_mir_transform/src/simplify.rs index 84905f4a400..6c8fa1fe9b3 100644 --- a/compiler/rustc_mir_transform/src/simplify.rs +++ b/compiler/rustc_mir_transform/src/simplify.rs @@ -26,6 +26,13 @@ //! Here the block (`{ return; }`) has the return type `char`, rather than `()`, but the MIR we //! naively generate still contains the `_a = ()` write in the unreachable block "after" the //! return. +//! +//! **WARNING**: This is one of the few optimizations that runs on built and analysis MIR, and +//! so its effects may affect the type-checking, borrow-checking, and other analysis of MIR. +//! We must be extremely careful to only apply optimizations that preserve UB and all +//! non-determinism, since changes here can affect which programs compile in an insta-stable way. +//! The normal logic that a program with UB can be changed to do anything does not apply to +//! pre-"runtime" MIR! use rustc_index::{Idx, IndexSlice, IndexVec}; use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor}; @@ -66,8 +73,8 @@ impl SimplifyCfg { } } -pub(super) fn simplify_cfg(body: &mut Body<'_>) { - CfgSimplifier::new(body).simplify(); +pub(super) fn simplify_cfg<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + CfgSimplifier::new(tcx, body).simplify(); remove_dead_blocks(body); // FIXME: Should probably be moved into some kind of pass manager @@ -79,9 +86,9 @@ impl<'tcx> crate::MirPass<'tcx> for SimplifyCfg { self.name() } - fn run_pass(&self, _: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { debug!("SimplifyCfg({:?}) - simplifying {:?}", self.name(), body.source); - simplify_cfg(body); + simplify_cfg(tcx, body); } fn is_required(&self) -> bool { @@ -90,12 +97,13 @@ impl<'tcx> crate::MirPass<'tcx> for SimplifyCfg { } struct CfgSimplifier<'a, 'tcx> { + preserve_switch_reads: bool, basic_blocks: &'a mut IndexSlice>, pred_count: IndexVec, } impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> { - fn new(body: &'a mut Body<'tcx>) -> Self { + fn new(tcx: TyCtxt<'tcx>, body: &'a mut Body<'tcx>) -> Self { let mut pred_count = IndexVec::from_elem(0u32, &body.basic_blocks); // we can't use mir.predecessors() here because that counts @@ -110,9 +118,12 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> { } } + // Preserve `SwitchInt` reads on built and analysis MIR, or if `-Zmir-preserve-ub`. + let preserve_switch_reads = matches!(body.phase, MirPhase::Built | MirPhase::Analysis(_)) + || tcx.sess.opts.unstable_opts.mir_preserve_ub; let basic_blocks = body.basic_blocks_mut(); - CfgSimplifier { basic_blocks, pred_count } + CfgSimplifier { preserve_switch_reads, basic_blocks, pred_count } } fn simplify(mut self) { @@ -253,9 +264,15 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> { // turn a branch with all successors identical to a goto fn simplify_branch(&mut self, terminator: &mut Terminator<'tcx>) -> bool { - match terminator.kind { - TerminatorKind::SwitchInt { .. } => {} - _ => return false, + // Removing a `SwitchInt` terminator may remove reads that result in UB, + // so we must not apply this optimization before borrowck or when + // `-Zmir-preserve-ub` is set. + if self.preserve_switch_reads { + return false; + } + + let TerminatorKind::SwitchInt { .. } = terminator.kind else { + return false; }; let first_succ = { diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index c70f1500d39..ea959d4ced1 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -2319,12 +2319,12 @@ options! { mir_include_spans: MirIncludeSpans = (MirIncludeSpans::default(), parse_mir_include_spans, [UNTRACKED], "include extra comments in mir pretty printing, like line numbers and statement indices, \ details about types, etc. (boolean for all passes, 'nll' to enable in NLL MIR only, default: 'nll')"), - mir_keep_place_mention: bool = (false, parse_bool, [TRACKED], - "keep place mention MIR statements, interpreted e.g., by miri; implies -Zmir-opt-level=0 \ - (default: no)"), #[rustc_lint_opt_deny_field_access("use `Session::mir_opt_level` instead of this field")] mir_opt_level: Option = (None, parse_opt_number, [TRACKED], "MIR optimization level (0-4; default: 1 in non optimized builds and 2 in optimized builds)"), + mir_preserve_ub: bool = (false, parse_bool, [TRACKED], + "keep place mention statements and reads in trivial SwitchInt terminators, which are interpreted \ + e.g., by miri; implies -Zmir-opt-level=0 (default: no)"), mir_strip_debuginfo: MirStripDebugInfo = (MirStripDebugInfo::None, parse_mir_strip_debuginfo, [TRACKED], "Whether to remove some of the MIR debug info from methods. Default: None"), move_size_limit: Option = (None, parse_opt_number, [TRACKED], diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 03f76cfa652..6f227e04d00 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -169,7 +169,7 @@ pub const MIRI_DEFAULT_ARGS: &[&str] = &[ "-Zalways-encode-mir", "-Zextra-const-ub-checks", "-Zmir-emit-retag", - "-Zmir-keep-place-mention", + "-Zmir-preserve-ub", "-Zmir-opt-level=0", "-Zmir-enable-passes=-CheckAlignment,-CheckNull", // Deduplicating diagnostics means we miss events when tracking what happens during an diff --git a/src/tools/miri/tests/fail/read_from_trivial_switch.rs b/src/tools/miri/tests/fail/read_from_trivial_switch.rs new file mode 100644 index 00000000000..d34b1cd5820 --- /dev/null +++ b/src/tools/miri/tests/fail/read_from_trivial_switch.rs @@ -0,0 +1,14 @@ +// Ensure that we don't optimize out `SwitchInt` reads even if that terminator +// branches to the same basic block on every target, since the operand may have +// side-effects that affect analysis of the MIR. +// +// See . + +use std::mem::MaybeUninit; + +fn main() { + let uninit: MaybeUninit = MaybeUninit::uninit(); + let bad_ref: &i32 = unsafe { uninit.assume_init_ref() }; + let &(0 | _) = bad_ref; + //~^ ERROR: Undefined Behavior: using uninitialized data, but this operation requires initialized memory +} diff --git a/src/tools/miri/tests/fail/read_from_trivial_switch.stderr b/src/tools/miri/tests/fail/read_from_trivial_switch.stderr new file mode 100644 index 00000000000..6b3d4539b96 --- /dev/null +++ b/src/tools/miri/tests/fail/read_from_trivial_switch.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory + --> tests/fail/read_from_trivial_switch.rs:LL:CC + | +LL | let &(0 | _) = bad_ref; + | ^^^^^^^^ using uninitialized data, but this operation requires initialized memory + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `main` at tests/fail/read_from_trivial_switch.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir index d52241b459e..2a965fe67b9 100644 --- a/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir @@ -24,43 +24,47 @@ fn match_tuple(_1: (u32, bool, Option, u32)) -> u32 { bb1: { _0 = const 0_u32; - goto -> bb10; + goto -> bb11; } bb2: { - _2 = discriminant((_1.2: std::option::Option)); - switchInt(move _2) -> [0: bb4, 1: bb3, otherwise: bb1]; + switchInt(copy (_1.1: bool)) -> [0: bb3, otherwise: bb3]; } bb3: { - switchInt(copy (((_1.2: std::option::Option) as Some).0: i32)) -> [1: bb4, 8: bb4, otherwise: bb1]; + _2 = discriminant((_1.2: std::option::Option)); + switchInt(move _2) -> [0: bb5, 1: bb4, otherwise: bb1]; } bb4: { - _5 = Le(const 6_u32, copy (_1.3: u32)); - switchInt(move _5) -> [0: bb5, otherwise: bb7]; + switchInt(copy (((_1.2: std::option::Option) as Some).0: i32)) -> [1: bb5, 8: bb5, otherwise: bb1]; } bb5: { - _3 = Le(const 13_u32, copy (_1.3: u32)); - switchInt(move _3) -> [0: bb1, otherwise: bb6]; + _5 = Le(const 6_u32, copy (_1.3: u32)); + switchInt(move _5) -> [0: bb6, otherwise: bb8]; } bb6: { - _4 = Le(copy (_1.3: u32), const 16_u32); - switchInt(move _4) -> [0: bb1, otherwise: bb8]; + _3 = Le(const 13_u32, copy (_1.3: u32)); + switchInt(move _3) -> [0: bb1, otherwise: bb7]; } bb7: { - _6 = Le(copy (_1.3: u32), const 9_u32); - switchInt(move _6) -> [0: bb5, otherwise: bb8]; + _4 = Le(copy (_1.3: u32), const 16_u32); + switchInt(move _4) -> [0: bb1, otherwise: bb9]; } bb8: { - falseEdge -> [real: bb9, imaginary: bb1]; + _6 = Le(copy (_1.3: u32), const 9_u32); + switchInt(move _6) -> [0: bb6, otherwise: bb9]; } bb9: { + falseEdge -> [real: bb10, imaginary: bb1]; + } + + bb10: { StorageLive(_7); _7 = copy (_1.0: u32); StorageLive(_8); @@ -74,10 +78,10 @@ fn match_tuple(_1: (u32, bool, Option, u32)) -> u32 { StorageDead(_9); StorageDead(_8); StorageDead(_7); - goto -> bb10; + goto -> bb11; } - bb10: { + bb11: { return; } } diff --git a/tests/mir-opt/dead-store-elimination/place_mention.rs b/tests/mir-opt/dead-store-elimination/place_mention.rs index 5e4a286a208..1848a028297 100644 --- a/tests/mir-opt/dead-store-elimination/place_mention.rs +++ b/tests/mir-opt/dead-store-elimination/place_mention.rs @@ -2,7 +2,7 @@ // and don't remove it as a dead store. // //@ test-mir-pass: DeadStoreElimination-initial -//@ compile-flags: -Zmir-keep-place-mention +//@ compile-flags: -Zmir-preserve-ub // EMIT_MIR place_mention.main.DeadStoreElimination-initial.diff fn main() { diff --git a/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir b/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir index 889ff6f9f5e..be0931eaa61 100644 --- a/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir @@ -14,7 +14,7 @@ fn single_switchint() -> () { } bb1: { - switchInt(copy (_2.0: i32)) -> [3: bb8, 4: bb8, otherwise: bb7]; + switchInt(copy (_2.0: i32)) -> [3: bb9, 4: bb9, otherwise: bb8]; } bb2: { @@ -22,7 +22,7 @@ fn single_switchint() -> () { } bb3: { - falseEdge -> [real: bb12, imaginary: bb4]; + falseEdge -> [real: bb14, imaginary: bb4]; } bb4: { @@ -30,43 +30,51 @@ fn single_switchint() -> () { } bb5: { - falseEdge -> [real: bb11, imaginary: bb6]; + falseEdge -> [real: bb13, imaginary: bb6]; } bb6: { - falseEdge -> [real: bb10, imaginary: bb1]; + switchInt(copy (_2.1: bool)) -> [0: bb7, otherwise: bb7]; } bb7: { - _1 = const 5_i32; - goto -> bb13; + falseEdge -> [real: bb12, imaginary: bb1]; } bb8: { - falseEdge -> [real: bb9, imaginary: bb7]; + _1 = const 5_i32; + goto -> bb15; } bb9: { - _1 = const 4_i32; - goto -> bb13; + switchInt(copy (_2.1: bool)) -> [0: bb10, otherwise: bb10]; } bb10: { - _1 = const 3_i32; - goto -> bb13; + falseEdge -> [real: bb11, imaginary: bb8]; } bb11: { - _1 = const 2_i32; - goto -> bb13; + _1 = const 4_i32; + goto -> bb15; } bb12: { - _1 = const 1_i32; - goto -> bb13; + _1 = const 3_i32; + goto -> bb15; } bb13: { + _1 = const 2_i32; + goto -> bb15; + } + + bb14: { + _1 = const 1_i32; + goto -> bb15; + } + + bb15: { StorageDead(_2); StorageDead(_1); _0 = const (); diff --git a/tests/mir-opt/read_from_trivial_switch.main.SimplifyCfg-initial.diff b/tests/mir-opt/read_from_trivial_switch.main.SimplifyCfg-initial.diff new file mode 100644 index 00000000000..87758408a1c --- /dev/null +++ b/tests/mir-opt/read_from_trivial_switch.main.SimplifyCfg-initial.diff @@ -0,0 +1,49 @@ +- // MIR for `main` before SimplifyCfg-initial ++ // MIR for `main` after SimplifyCfg-initial + + fn main() -> () { + let mut _0: (); + let _1: &i32; + let _2: i32; + scope 1 { + debug ref_ => _1; + scope 2 { + } + } + + bb0: { + StorageLive(_1); + StorageLive(_2); + _2 = const 1_i32; + _1 = &_2; + FakeRead(ForLet(None), _1); + PlaceMention(_1); +- switchInt(copy (*_1)) -> [0: bb2, otherwise: bb1]; ++ switchInt(copy (*_1)) -> [0: bb1, otherwise: bb1]; + } + + bb1: { +- goto -> bb5; +- } +- +- bb2: { +- goto -> bb5; +- } +- +- bb3: { +- goto -> bb1; +- } +- +- bb4: { +- FakeRead(ForMatchedPlace(None), _1); +- unreachable; +- } +- +- bb5: { + _0 = const (); + StorageDead(_2); + StorageDead(_1); + return; + } + } + diff --git a/tests/mir-opt/read_from_trivial_switch.rs b/tests/mir-opt/read_from_trivial_switch.rs new file mode 100644 index 00000000000..1c64c1d45e8 --- /dev/null +++ b/tests/mir-opt/read_from_trivial_switch.rs @@ -0,0 +1,15 @@ +// Ensure that we don't optimize out `SwitchInt` reads even if that terminator +// branches to the same basic block on every target, since the operand may have +// side-effects that affect analysis of the MIR. +// +// See . + +//@ test-mir-pass: SimplifyCfg-initial +//@ compile-flags: -Zmir-preserve-ub + +// EMIT_MIR read_from_trivial_switch.main.SimplifyCfg-initial.diff +fn main() { + let ref_ = &1i32; + // CHECK: switchInt + let &(0 | _) = ref_; +} diff --git a/tests/ui/pattern/uninit-trivial.rs b/tests/ui/pattern/uninit-trivial.rs new file mode 100644 index 00000000000..6ea6796c1c1 --- /dev/null +++ b/tests/ui/pattern/uninit-trivial.rs @@ -0,0 +1,8 @@ +// Regression test for the semantic changes in +// . + +fn main() { + let x; + let (0 | _) = x; + //~^ ERROR used binding `x` isn't initialized +} diff --git a/tests/ui/pattern/uninit-trivial.stderr b/tests/ui/pattern/uninit-trivial.stderr new file mode 100644 index 00000000000..2ff8557c945 --- /dev/null +++ b/tests/ui/pattern/uninit-trivial.stderr @@ -0,0 +1,16 @@ +error[E0381]: used binding `x` isn't initialized + --> $DIR/uninit-trivial.rs:6:10 + | +LL | let x; + | - binding declared here but left uninitialized +LL | let (0 | _) = x; + | ^^^^^ `x` used here but it isn't initialized + | +help: consider assigning a value + | +LL | let x = 42; + | ++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0381`. From 199ee4084349361ae44c6e4b0cbc9a5e72913b20 Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Tue, 25 Mar 2025 14:48:12 +0100 Subject: [PATCH 04/67] Move errors --- .../src/error_reporting/traits/mod.rs | 1 + .../traits/on_unimplemented.rs | 79 +---------------- .../traits/on_unimplemented_format.rs | 86 +++++++++++++++++++ 3 files changed, 89 insertions(+), 77 deletions(-) create mode 100644 compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs index 8ff7030717a..7fd1c0d2743 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs @@ -2,6 +2,7 @@ pub mod ambiguity; pub mod call_kind; mod fulfillment_errors; pub mod on_unimplemented; +pub mod on_unimplemented_format; mod overflow; pub mod suggestions; diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs index f0c6e51f2a4..9914e828a93 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs @@ -7,18 +7,18 @@ use rustc_errors::codes::*; use rustc_errors::{ErrorGuaranteed, struct_span_code_err}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::{AttrArgs, Attribute}; -use rustc_macros::LintDiagnostic; use rustc_middle::bug; use rustc_middle::ty::print::PrintTraitRefExt as _; use rustc_middle::ty::{self, GenericArgsRef, GenericParamDefKind, TyCtxt}; use rustc_parse_format::{ParseMode, Parser, Piece, Position}; use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; -use rustc_span::{Ident, Span, Symbol, kw, sym}; +use rustc_span::{Span, Symbol, kw, sym}; use tracing::{debug, info}; use {rustc_attr_parsing as attr, rustc_hir as hir}; use super::{ObligationCauseCode, PredicateObligation}; use crate::error_reporting::TypeErrCtxt; +use crate::error_reporting::traits::on_unimplemented_format::errors::*; use crate::errors::{ EmptyOnClauseInOnUnimplemented, InvalidOnClauseInOnUnimplemented, NoValueInOnUnimplemented, }; @@ -320,81 +320,6 @@ pub enum AppendConstMessage { Custom(Symbol, Span), } -#[derive(LintDiagnostic)] -#[diag(trait_selection_malformed_on_unimplemented_attr)] -#[help] -pub struct MalformedOnUnimplementedAttrLint { - #[label] - pub span: Span, -} - -impl MalformedOnUnimplementedAttrLint { - fn new(span: Span) -> Self { - Self { span } - } -} - -#[derive(LintDiagnostic)] -#[diag(trait_selection_missing_options_for_on_unimplemented_attr)] -#[help] -pub struct MissingOptionsForOnUnimplementedAttr; - -#[derive(LintDiagnostic)] -#[diag(trait_selection_ignored_diagnostic_option)] -pub struct IgnoredDiagnosticOption { - pub option_name: &'static str, - #[label] - pub span: Span, - #[label(trait_selection_other_label)] - pub prev_span: Span, -} - -impl IgnoredDiagnosticOption { - fn maybe_emit_warning<'tcx>( - tcx: TyCtxt<'tcx>, - item_def_id: DefId, - new: Option, - old: Option, - option_name: &'static str, - ) { - if let (Some(new_item), Some(old_item)) = (new, old) { - if let Some(item_def_id) = item_def_id.as_local() { - tcx.emit_node_span_lint( - UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, - tcx.local_def_id_to_hir_id(item_def_id), - new_item, - IgnoredDiagnosticOption { span: new_item, prev_span: old_item, option_name }, - ); - } - } - } -} - -#[derive(LintDiagnostic)] -#[diag(trait_selection_unknown_format_parameter_for_on_unimplemented_attr)] -#[help] -pub struct UnknownFormatParameterForOnUnimplementedAttr { - argument_name: Symbol, - trait_name: Ident, -} - -#[derive(LintDiagnostic)] -#[diag(trait_selection_disallowed_positional_argument)] -#[help] -pub struct DisallowedPositionalArgument; - -#[derive(LintDiagnostic)] -#[diag(trait_selection_invalid_format_specifier)] -#[help] -pub struct InvalidFormatSpecifier; - -#[derive(LintDiagnostic)] -#[diag(trait_selection_wrapped_parser_error)] -pub struct WrappedParserError { - description: String, - label: String, -} - impl<'tcx> OnUnimplementedDirective { fn parse( tcx: TyCtxt<'tcx>, diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs new file mode 100644 index 00000000000..40a47921fc1 --- /dev/null +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs @@ -0,0 +1,86 @@ +pub mod errors { + use rustc_macros::LintDiagnostic; + use rustc_middle::ty::TyCtxt; + use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; + use rustc_span::Ident; + + use super::*; + + #[derive(LintDiagnostic)] + #[diag(trait_selection_unknown_format_parameter_for_on_unimplemented_attr)] + #[help] + pub struct UnknownFormatParameterForOnUnimplementedAttr { + pub argument_name: Symbol, + pub trait_name: Ident, + } + + #[derive(LintDiagnostic)] + #[diag(trait_selection_disallowed_positional_argument)] + #[help] + pub struct DisallowedPositionalArgument; + + #[derive(LintDiagnostic)] + #[diag(trait_selection_invalid_format_specifier)] + #[help] + pub struct InvalidFormatSpecifier; + + #[derive(LintDiagnostic)] + #[diag(trait_selection_wrapped_parser_error)] + pub struct WrappedParserError { + pub description: String, + pub label: String, + } + #[derive(LintDiagnostic)] + #[diag(trait_selection_malformed_on_unimplemented_attr)] + #[help] + pub struct MalformedOnUnimplementedAttrLint { + #[label] + pub span: Span, + } + + impl MalformedOnUnimplementedAttrLint { + pub fn new(span: Span) -> Self { + Self { span } + } + } + + #[derive(LintDiagnostic)] + #[diag(trait_selection_missing_options_for_on_unimplemented_attr)] + #[help] + pub struct MissingOptionsForOnUnimplementedAttr; + + #[derive(LintDiagnostic)] + #[diag(trait_selection_ignored_diagnostic_option)] + pub struct IgnoredDiagnosticOption { + pub option_name: &'static str, + #[label] + pub span: Span, + #[label(trait_selection_other_label)] + pub prev_span: Span, + } + + impl IgnoredDiagnosticOption { + pub fn maybe_emit_warning<'tcx>( + tcx: TyCtxt<'tcx>, + item_def_id: DefId, + new: Option, + old: Option, + option_name: &'static str, + ) { + if let (Some(new_item), Some(old_item)) = (new, old) { + if let Some(item_def_id) = item_def_id.as_local() { + tcx.emit_node_span_lint( + UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, + tcx.local_def_id_to_hir_id(item_def_id), + new_item, + IgnoredDiagnosticOption { + span: new_item, + prev_span: old_item, + option_name, + }, + ); + } + } + } + } +} From 2007c8994db6d46533f84f4697fea5734c6df53d Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Thu, 27 Mar 2025 15:39:23 +0100 Subject: [PATCH 05/67] Write the format string parserand split it from conditions parser --- compiler/rustc_span/src/symbol.rs | 1 + .../src/error_reporting/traits/mod.rs | 1 + .../traits/on_unimplemented.rs | 399 ++++++------------ .../traits/on_unimplemented_condition.rs | 64 +++ .../traits/on_unimplemented_format.rs | 344 +++++++++++++++ .../on_unimplemented/broken_format.stderr | 48 +-- ...ons_of_the_internal_rustc_attribute.stderr | 80 ++-- ...t_fail_parsing_on_invalid_options_1.stderr | 8 +- tests/ui/on-unimplemented/bad-annotation.rs | 4 +- .../ui/on-unimplemented/bad-annotation.stderr | 22 +- tests/ui/on-unimplemented/impl-substs.stderr | 2 +- 11 files changed, 613 insertions(+), 360 deletions(-) create mode 100644 compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 31847ae3b46..0c458db0b23 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -372,6 +372,7 @@ symbols! { SyncUnsafeCell, T, Target, + This, ToOwned, ToString, TokenStream, diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs index 7fd1c0d2743..78f9287b407 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs @@ -2,6 +2,7 @@ pub mod ambiguity; pub mod call_kind; mod fulfillment_errors; pub mod on_unimplemented; +pub mod on_unimplemented_condition; pub mod on_unimplemented_format; mod overflow; pub mod suggestions; diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs index 9914e828a93..6a7f3223421 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs @@ -1,7 +1,7 @@ use std::iter; use std::path::PathBuf; -use rustc_ast::MetaItemInner; +use rustc_ast::{LitKind, MetaItem, MetaItemInner, MetaItemKind, MetaItemLit}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::codes::*; use rustc_errors::{ErrorGuaranteed, struct_span_code_err}; @@ -10,35 +10,21 @@ use rustc_hir::{AttrArgs, Attribute}; use rustc_middle::bug; use rustc_middle::ty::print::PrintTraitRefExt as _; use rustc_middle::ty::{self, GenericArgsRef, GenericParamDefKind, TyCtxt}; -use rustc_parse_format::{ParseMode, Parser, Piece, Position}; use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; -use rustc_span::{Span, Symbol, kw, sym}; +use rustc_span::{Span, Symbol, sym}; use tracing::{debug, info}; use {rustc_attr_parsing as attr, rustc_hir as hir}; use super::{ObligationCauseCode, PredicateObligation}; use crate::error_reporting::TypeErrCtxt; +use crate::error_reporting::traits::on_unimplemented_condition::Condition; use crate::error_reporting::traits::on_unimplemented_format::errors::*; +use crate::error_reporting::traits::on_unimplemented_format::{Ctx, FormatString}; use crate::errors::{ EmptyOnClauseInOnUnimplemented, InvalidOnClauseInOnUnimplemented, NoValueInOnUnimplemented, }; use crate::infer::InferCtxtExt; -/// The symbols which are always allowed in a format string -static ALLOWED_FORMAT_SYMBOLS: &[Symbol] = &[ - kw::SelfUpper, - sym::ItemContext, - sym::from_desugaring, - sym::direct, - sym::cause, - sym::integral, - sym::integer_, - sym::float, - sym::_Self, - sym::crate_local, - sym::Trait, -]; - impl<'tcx> TypeErrCtxt<'_, 'tcx> { fn impl_similar_to( &self, @@ -275,6 +261,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } })); + flags.push((sym::This, Some(self.tcx.def_path_str(trait_pred.trait_ref.def_id)))); + if let Ok(Some(command)) = OnUnimplementedDirective::of_item(self.tcx, def_id) { command.evaluate(self.tcx, trait_pred.trait_ref, &flags, long_ty_file) } else { @@ -283,19 +271,23 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } } +/// Represents a format string in a on_unimplemented attribute, +/// like the "content" in `#[diagnostic::on_unimplemented(message = "content")]` #[derive(Clone, Debug)] pub struct OnUnimplementedFormatString { - symbol: Symbol, - span: Span, - is_diagnostic_namespace_variant: bool, + /// Symbol of the format string, i.e. `"content"` + pub symbol: Symbol, + ///The span of the format string, i.e. `"content"` + pub span: Span, + pub is_diagnostic_namespace_variant: bool, } #[derive(Debug)] pub struct OnUnimplementedDirective { - pub condition: Option, + pub condition: Option, pub subcommands: Vec, - pub message: Option, - pub label: Option, + pub message: Option<(Span, OnUnimplementedFormatString)>, + pub label: Option<(Span, OnUnimplementedFormatString)>, pub notes: Vec, pub parent_label: Option, pub append_const_msg: Option, @@ -332,12 +324,12 @@ impl<'tcx> OnUnimplementedDirective { let mut errored = None; let mut item_iter = items.iter(); - let parse_value = |value_str, value_span| { + let parse_value = |value_str, span| { OnUnimplementedFormatString::try_parse( tcx, item_def_id, value_str, - value_span, + span, is_diagnostic_namespace_variant, ) .map(Some) @@ -359,7 +351,7 @@ impl<'tcx> OnUnimplementedDirective { } true }); - Some(cond.clone()) + Some(Condition { inner: cond.clone() }) }; let mut message = None; @@ -369,24 +361,36 @@ impl<'tcx> OnUnimplementedDirective { let mut subcommands = vec![]; let mut append_const_msg = None; + let get_value_and_span = |item: &_, key| { + if let MetaItemInner::MetaItem(MetaItem { + path, + kind: MetaItemKind::NameValue(MetaItemLit { span, kind: LitKind::Str(s, _), .. }), + .. + }) = item + && *path == key + { + Some((*s, *span)) + } else { + None + } + }; + for item in item_iter { - if item.has_name(sym::message) && message.is_none() { - if let Some(message_) = item.value_str() { - message = parse_value(message_, item.span())?; + if let Some((message_, span)) = get_value_and_span(item, sym::message) + && message.is_none() + { + message = parse_value(message_, span)?.map(|l| (item.span(), l)); + continue; + } else if let Some((label_, span)) = get_value_and_span(item, sym::label) + && label.is_none() + { + label = parse_value(label_, span)?.map(|l| (item.span(), l)); + continue; + } else if let Some((note_, span)) = get_value_and_span(item, sym::note) { + if let Some(note) = parse_value(note_, span)? { + notes.push(note); continue; } - } else if item.has_name(sym::label) && label.is_none() { - if let Some(label_) = item.value_str() { - label = parse_value(label_, item.span())?; - continue; - } - } else if item.has_name(sym::note) { - if let Some(note_) = item.value_str() { - if let Some(note) = parse_value(note_, item.span())? { - notes.push(note); - continue; - } - } } else if item.has_name(sym::parent_label) && parent_label.is_none() && !is_diagnostic_namespace_variant @@ -479,15 +483,15 @@ impl<'tcx> OnUnimplementedDirective { IgnoredDiagnosticOption::maybe_emit_warning( tcx, item_def_id, - directive.message.as_ref().map(|f| f.span), - aggr.message.as_ref().map(|f| f.span), + directive.message.as_ref().map(|f| f.0), + aggr.message.as_ref().map(|f| f.0), "message", ); IgnoredDiagnosticOption::maybe_emit_warning( tcx, item_def_id, - directive.label.as_ref().map(|f| f.span), - aggr.label.as_ref().map(|f| f.span), + directive.label.as_ref().map(|f| f.0), + aggr.label.as_ref().map(|f| f.0), "label", ); IgnoredDiagnosticOption::maybe_emit_warning( @@ -561,13 +565,16 @@ impl<'tcx> OnUnimplementedDirective { condition: None, message: None, subcommands: vec![], - label: Some(OnUnimplementedFormatString::try_parse( - tcx, - item_def_id, - value, + label: Some(( attr.span(), - is_diagnostic_namespace_variant, - )?), + OnUnimplementedFormatString::try_parse( + tcx, + item_def_id, + value, + attr.value_span().unwrap_or(attr.span()), + is_diagnostic_namespace_variant, + )?, + )), notes: Vec::new(), parent_label: None, append_const_msg: None, @@ -643,27 +650,7 @@ impl<'tcx> OnUnimplementedDirective { for command in self.subcommands.iter().chain(Some(self)).rev() { debug!(?command); if let Some(ref condition) = command.condition - && !attr::eval_condition(condition, &tcx.sess, Some(tcx.features()), &mut |cfg| { - let value = cfg.value.map(|v| { - // `with_no_visible_paths` is also used when generating the options, - // so we need to match it here. - ty::print::with_no_visible_paths!( - OnUnimplementedFormatString { - symbol: v, - span: cfg.span, - is_diagnostic_namespace_variant: false - } - .format( - tcx, - trait_ref, - &options_map, - long_ty_file - ) - ) - }); - - options.contains(&(cfg.name, value)) - }) + && !condition.matches_predicate(tcx, options, &options_map) { debug!("evaluate: skipping {:?} due to condition", command); continue; @@ -687,8 +674,8 @@ impl<'tcx> OnUnimplementedDirective { } OnUnimplementedNote { - label: label.map(|l| l.format(tcx, trait_ref, &options_map, long_ty_file)), - message: message.map(|m| m.format(tcx, trait_ref, &options_map, long_ty_file)), + label: label.map(|l| l.1.format(tcx, trait_ref, &options_map, long_ty_file)), + message: message.map(|m| m.1.format(tcx, trait_ref, &options_map, long_ty_file)), notes: notes .into_iter() .map(|n| n.format(tcx, trait_ref, &options_map, long_ty_file)) @@ -705,144 +692,65 @@ impl<'tcx> OnUnimplementedFormatString { tcx: TyCtxt<'tcx>, item_def_id: DefId, from: Symbol, - value_span: Span, + span: Span, is_diagnostic_namespace_variant: bool, ) -> Result { - let result = OnUnimplementedFormatString { - symbol: from, - span: value_span, - is_diagnostic_namespace_variant, - }; + let result = + OnUnimplementedFormatString { symbol: from, span, is_diagnostic_namespace_variant }; result.verify(tcx, item_def_id)?; Ok(result) } fn verify(&self, tcx: TyCtxt<'tcx>, item_def_id: DefId) -> Result<(), ErrorGuaranteed> { - let trait_def_id = if tcx.is_trait(item_def_id) { - item_def_id + let trait_def_id = if tcx.is_trait(item_def_id) { item_def_id } else { return Ok(()) }; + + let ctx = if self.is_diagnostic_namespace_variant { + Ctx::DiagnosticOnUnimplemented { tcx, trait_def_id } } else { - tcx.trait_id_of_impl(item_def_id) - .expect("expected `on_unimplemented` to correspond to a trait") + Ctx::RustcOnUnimplemented { tcx, trait_def_id } }; - let trait_name = tcx.item_ident(trait_def_id); - let generics = tcx.generics_of(item_def_id); - let s = self.symbol.as_str(); - let mut parser = Parser::new(s, None, None, false, ParseMode::Format); + let mut result = Ok(()); - for token in &mut parser { - match token { - Piece::Lit(_) => (), // Normal string, no need to check it - Piece::NextArgument(a) => { - let format_spec = a.format; - if self.is_diagnostic_namespace_variant - && (format_spec.ty_span.is_some() - || format_spec.width_span.is_some() - || format_spec.precision_span.is_some() - || format_spec.fill_span.is_some()) - { + + match FormatString::parse(self.symbol, self.span, &ctx) { + // Warnings about format specifiers, deprecated parameters, wrong parameters etc. + // In other words we'd like to let the author know, but we can still try to format the string later + Ok(FormatString { warnings, .. }) => { + for w in warnings { + w.emit_warning(tcx, trait_def_id) + } + } + // Errors from the underlying `rustc_parse_format::Parser` + Err(errors) => { + // we cannot return errors from processing the format string as hard error here + // as the diagnostic namespace guarantees that malformed input cannot cause an error + // + // if we encounter any error while processing we nevertheless want to show it as warning + // so that users are aware that something is not correct + for e in errors { + if self.is_diagnostic_namespace_variant { if let Some(item_def_id) = item_def_id.as_local() { tcx.emit_node_span_lint( UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, tcx.local_def_id_to_hir_id(item_def_id), self.span, - InvalidFormatSpecifier, + WrappedParserError { description: e.description, label: e.label }, ); } - } - match a.position { - Position::ArgumentNamed(s) => { - match Symbol::intern(s) { - // `{ThisTraitsName}` is allowed - s if s == trait_name.name - && !self.is_diagnostic_namespace_variant => - { - () - } - s if ALLOWED_FORMAT_SYMBOLS.contains(&s) - && !self.is_diagnostic_namespace_variant => - { - () - } - // So is `{A}` if A is a type parameter - s if generics.own_params.iter().any(|param| param.name == s) => (), - s => { - if self.is_diagnostic_namespace_variant { - if let Some(item_def_id) = item_def_id.as_local() { - tcx.emit_node_span_lint( - UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, - tcx.local_def_id_to_hir_id(item_def_id), - self.span, - UnknownFormatParameterForOnUnimplementedAttr { - argument_name: s, - trait_name, - }, - ); - } - } else { - result = Err(struct_span_code_err!( - tcx.dcx(), - self.span, - E0230, - "there is no parameter `{}` on {}", - s, - if trait_def_id == item_def_id { - format!("trait `{trait_name}`") - } else { - "impl".to_string() - } - ) - .emit()); - } - } - } - } - // `{:1}` and `{}` are not to be used - Position::ArgumentIs(..) | Position::ArgumentImplicitlyIs(_) => { - if self.is_diagnostic_namespace_variant { - if let Some(item_def_id) = item_def_id.as_local() { - tcx.emit_node_span_lint( - UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, - tcx.local_def_id_to_hir_id(item_def_id), - self.span, - DisallowedPositionalArgument, - ); - } - } else { - let reported = struct_span_code_err!( - tcx.dcx(), - self.span, - E0231, - "only named generic parameters are allowed" - ) - .emit(); - result = Err(reported); - } - } + } else { + let reported = struct_span_code_err!( + tcx.dcx(), + self.span, + E0231, + "{}", + e.description, + ) + .emit(); + result = Err(reported); } } } } - // we cannot return errors from processing the format string as hard error here - // as the diagnostic namespace guarantees that malformed input cannot cause an error - // - // if we encounter any error while processing we nevertheless want to show it as warning - // so that users are aware that something is not correct - for e in parser.errors { - if self.is_diagnostic_namespace_variant { - if let Some(item_def_id) = item_def_id.as_local() { - tcx.emit_node_span_lint( - UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, - tcx.local_def_id_to_hir_id(item_def_id), - self.span, - WrappedParserError { description: e.description, label: e.label }, - ); - } - } else { - let reported = - struct_span_code_err!(tcx.dcx(), self.span, E0231, "{}", e.description,).emit(); - result = Err(reported); - } - } result } @@ -854,95 +762,26 @@ impl<'tcx> OnUnimplementedFormatString { options: &FxHashMap, long_ty_file: &mut Option, ) -> String { - let name = tcx.item_name(trait_ref.def_id); - let trait_str = tcx.def_path_str(trait_ref.def_id); - let generics = tcx.generics_of(trait_ref.def_id); - let generic_map = generics - .own_params - .iter() - .filter_map(|param| { - let value = match param.kind { - GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { - if let Some(ty) = trait_ref.args[param.index as usize].as_type() { - tcx.short_string(ty, long_ty_file) - } else { - trait_ref.args[param.index as usize].to_string() - } - } - GenericParamDefKind::Lifetime => return None, - }; - let name = param.name; - Some((name, value)) - }) - .collect::>(); - let empty_string = String::new(); - - let s = self.symbol.as_str(); - let mut parser = Parser::new(s, None, None, false, ParseMode::Format); - let item_context = (options.get(&sym::ItemContext)).unwrap_or(&empty_string); - let constructed_message = (&mut parser) - .map(|p| match p { - Piece::Lit(s) => s.to_owned(), - Piece::NextArgument(a) => match a.position { - Position::ArgumentNamed(arg) => { - let s = Symbol::intern(arg); - match generic_map.get(&s) { - Some(val) => val.to_string(), - None if self.is_diagnostic_namespace_variant => { - format!("{{{arg}}}") - } - None if s == name => trait_str.clone(), - None => { - if let Some(val) = options.get(&s) { - val.clone() - } else if s == sym::from_desugaring { - // don't break messages using these two arguments incorrectly - String::new() - } else if s == sym::ItemContext - && !self.is_diagnostic_namespace_variant - { - item_context.clone() - } else if s == sym::integral { - String::from("{integral}") - } else if s == sym::integer_ { - String::from("{integer}") - } else if s == sym::float { - String::from("{float}") - } else { - bug!( - "broken on_unimplemented {:?} for {:?}: \ - no argument matching {:?}", - self.symbol, - trait_ref, - s - ) - } - } - } - } - Position::ArgumentImplicitlyIs(_) if self.is_diagnostic_namespace_variant => { - String::from("{}") - } - Position::ArgumentIs(idx) if self.is_diagnostic_namespace_variant => { - format!("{{{idx}}}") - } - _ => bug!("broken on_unimplemented {:?} - bad format arg", self.symbol), - }, - }) - .collect(); - // we cannot return errors from processing the format string as hard error here - // as the diagnostic namespace guarantees that malformed input cannot cause an error - // - // if we encounter any error while processing the format string - // we don't want to show the potentially half assembled formatted string, - // therefore we fall back to just showing the input string in this case - // - // The actual parser errors are emitted earlier - // as lint warnings in OnUnimplementedFormatString::verify - if self.is_diagnostic_namespace_variant && !parser.errors.is_empty() { - String::from(s) + let trait_def_id = trait_ref.def_id; + let ctx = if self.is_diagnostic_namespace_variant { + Ctx::DiagnosticOnUnimplemented { tcx, trait_def_id } } else { - constructed_message + Ctx::RustcOnUnimplemented { tcx, trait_def_id } + }; + + if let Ok(s) = FormatString::parse(self.symbol, self.span, &ctx) { + s.format(tcx, trait_ref, options, long_ty_file) + } else { + // we cannot return errors from processing the format string as hard error here + // as the diagnostic namespace guarantees that malformed input cannot cause an error + // + // if we encounter any error while processing the format string + // we don't want to show the potentially half assembled formatted string, + // therefore we fall back to just showing the input string in this case + // + // The actual parser errors are emitted earlier + // as lint warnings in OnUnimplementedFormatString::verify + self.symbol.as_str().into() } } } diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs new file mode 100644 index 00000000000..4c852c1023a --- /dev/null +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs @@ -0,0 +1,64 @@ +use rustc_ast::MetaItemInner; +use rustc_attr_parsing as attr; +use rustc_data_structures::fx::FxHashMap; +use rustc_middle::ty::{self, TyCtxt}; +use rustc_parse_format::{ParseMode, Parser, Piece, Position}; +use rustc_span::{Span, Symbol, sym}; + +pub static ALLOWED_CONDITION_SYMBOLS: &[Symbol] = &[ + sym::from_desugaring, + sym::direct, + sym::cause, + sym::integral, + sym::integer_, + sym::float, + sym::_Self, + sym::crate_local, +]; + +#[derive(Debug)] +pub struct Condition { + pub inner: MetaItemInner, +} + +impl Condition { + pub fn span(&self) -> Span { + self.inner.span() + } + + pub fn matches_predicate<'tcx>( + &self, + tcx: TyCtxt<'tcx>, + options: &[(Symbol, Option)], + options_map: &FxHashMap, + ) -> bool { + attr::eval_condition(&self.inner, tcx.sess, Some(tcx.features()), &mut |cfg| { + let value = cfg.value.map(|v| { + // `with_no_visible_paths` is also used when generating the options, + // so we need to match it here. + ty::print::with_no_visible_paths!({ + let mut parser = Parser::new(v.as_str(), None, None, false, ParseMode::Format); + let constructed_message = (&mut parser) + .map(|p| match p { + Piece::Lit(s) => s.to_owned(), + Piece::NextArgument(a) => match a.position { + Position::ArgumentNamed(arg) => { + let s = Symbol::intern(arg); + match options_map.get(&s) { + Some(val) => val.to_string(), + None => format!("{{{arg}}}"), + } + } + Position::ArgumentImplicitlyIs(_) => String::from("{}"), + Position::ArgumentIs(idx) => format!("{{{idx}}}"), + }, + }) + .collect(); + constructed_message + }) + }); + + options.contains(&(cfg.name, value)) + }) + } +} diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs index 40a47921fc1..21db207496c 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs @@ -1,3 +1,347 @@ +use std::fmt::Write; +use std::path::PathBuf; + +use errors::*; +use rustc_data_structures::fx::FxHashMap; +use rustc_middle::span_bug; +use rustc_middle::ty::{self, GenericParamDefKind, TyCtxt}; +use rustc_parse_format::{ + Alignment, Argument, Count, FormatSpec, InnerSpan, ParseError, ParseMode, Parser, + Piece as RpfPiece, Position, +}; +use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; +use rustc_span::def_id::DefId; +use rustc_span::{BytePos, Pos, Span, Symbol, kw, sym}; + +pub struct FormatString { + input: Symbol, + input_span: Span, + pieces: Vec, + // the formatting string was parsed succesfully but with warnings + pub warnings: Vec, +} + +enum Piece { + Lit(String), + Arg(FormatArg), +} + +pub enum FormatArg { + // A generic parameter, like `{T}` if we're on the `From` trait. + GenericParam { generic_param: Symbol, span: Span }, + // `{Self}` + SelfUpper, + This, + Trait, + ItemContext, + AsIs(String), +} + +pub enum Ctx<'tcx> { + // `#[rustc_on_unimplemented]` + RustcOnUnimplemented { tcx: TyCtxt<'tcx>, trait_def_id: DefId }, + // `#[diagnostic::...]` + DiagnosticOnUnimplemented { tcx: TyCtxt<'tcx>, trait_def_id: DefId }, +} + +pub enum FormatWarning { + UnknownParam { argument_name: Symbol, span: Span }, + PositionalArgument { span: Span, help: String }, + InvalidSpecifier { name: String, span: Span }, + FutureIncompat { span: Span, help: String }, +} + +impl FormatWarning { + pub fn emit_warning<'tcx>(&self, tcx: TyCtxt<'tcx>, item_def_id: DefId) { + match *self { + FormatWarning::UnknownParam { argument_name, span } => { + let this = tcx.item_ident(item_def_id); + if let Some(item_def_id) = item_def_id.as_local() { + tcx.emit_node_span_lint( + UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, + tcx.local_def_id_to_hir_id(item_def_id), + span, + UnknownFormatParameterForOnUnimplementedAttr { + argument_name, + trait_name: this, + }, + ); + } + } + FormatWarning::PositionalArgument { span, .. } => { + if let Some(item_def_id) = item_def_id.as_local() { + tcx.emit_node_span_lint( + UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, + tcx.local_def_id_to_hir_id(item_def_id), + span, + DisallowedPositionalArgument, + ); + } + } + FormatWarning::InvalidSpecifier { span, .. } => { + if let Some(item_def_id) = item_def_id.as_local() { + tcx.emit_node_span_lint( + UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, + tcx.local_def_id_to_hir_id(item_def_id), + span, + InvalidFormatSpecifier, + ); + } + } + FormatWarning::FutureIncompat { .. } => { + // We've never deprecated anything in diagnostic namespace format strings + // but if we do we will emit a warning here + + // FIXME(mejrs) in a couple releases, start emitting warnings for + // #[rustc_on_unimplemented] deprecated args + } + } + } +} + +impl FormatString { + pub fn parse(input: Symbol, input_span: Span, ctx: &Ctx<'_>) -> Result> { + let s = input.as_str(); + let mut parser = Parser::new(s, None, None, false, ParseMode::Format); + let mut pieces = Vec::new(); + let mut warnings = Vec::new(); + + for piece in &mut parser { + match piece { + RpfPiece::Lit(lit) => { + pieces.push(Piece::Lit(lit.into())); + } + RpfPiece::NextArgument(arg) => { + warn_on_format_spec(arg.format, &mut warnings, input_span); + let arg = parse_arg(&arg, ctx, &mut warnings, input_span); + pieces.push(Piece::Arg(arg)); + } + } + } + + if parser.errors.is_empty() { + Ok(FormatString { input, input_span, pieces, warnings }) + } else { + Err(parser.errors) + } + } + + pub fn format<'tcx>( + &self, + tcx: TyCtxt<'tcx>, + trait_ref: ty::TraitRef<'tcx>, + options: &FxHashMap, + long_ty_file: &mut Option, + ) -> String { + let generics = tcx.generics_of(trait_ref.def_id); + let generic_map = generics + .own_params + .iter() + .filter_map(|param| { + let value = match param.kind { + GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { + if let Some(ty) = trait_ref.args[param.index as usize].as_type() { + tcx.short_string(ty, long_ty_file) + } else { + trait_ref.args[param.index as usize].to_string() + } + } + GenericParamDefKind::Lifetime => return None, + }; + let name = param.name; + Some((name, value)) + }) + .collect::>(); + + let mut ret = String::new(); + for piece in &self.pieces { + match piece { + Piece::Lit(s) | Piece::Arg(FormatArg::AsIs(s)) => ret.push_str(&s), + + // `A` if we have `trait Trait {}` and `note = "i'm the actual type of {A}"` + Piece::Arg(FormatArg::GenericParam { generic_param, span }) => { + // Should always be some but we can't raise errors here + if let Some(value) = generic_map.get(&generic_param) { + ret.push_str(value); + } else if cfg!(debug_assertions) { + span_bug!(*span, "invalid generic parameter"); + } else { + let _ = ret.write_fmt(format_args!("{{{}}}", generic_param.as_str())); + } + } + // `{Self}` + Piece::Arg(FormatArg::SelfUpper) => { + let Some(slf) = generic_map.get(&kw::SelfUpper) else { + span_bug!( + self.input_span, + "broken format string {:?} for {:?}: \ + no argument matching `Self`", + self.input, + trait_ref, + ) + }; + ret.push_str(&slf); + } + + // It's only `rustc_onunimplemented` from here + Piece::Arg(FormatArg::This) => { + let Some(this) = options.get(&sym::This) else { + span_bug!( + self.input_span, + "broken format string {:?} for {:?}: \ + no argument matching This", + self.input, + trait_ref, + ) + }; + ret.push_str(this); + } + Piece::Arg(FormatArg::Trait) => { + let Some(this) = options.get(&sym::Trait) else { + span_bug!( + self.input_span, + "broken format string {:?} for {:?}: \ + no argument matching Trait", + self.input, + trait_ref, + ) + }; + ret.push_str(this); + } + Piece::Arg(FormatArg::ItemContext) => { + let itemcontext = options.get(&sym::ItemContext); + ret.push_str(itemcontext.unwrap_or(&String::new())); + } + } + } + ret + } +} + +fn parse_arg( + arg: &Argument<'_>, + ctx: &Ctx<'_>, + warnings: &mut Vec, + input_span: Span, +) -> FormatArg { + let (Ctx::RustcOnUnimplemented { tcx, trait_def_id } + | Ctx::DiagnosticOnUnimplemented { tcx, trait_def_id }) = ctx; + let trait_name = tcx.item_ident(*trait_def_id); + let generics = tcx.generics_of(trait_def_id); + let span = slice_span(input_span, arg.position_span); + + match arg.position { + // Something like "hello {name}" + Position::ArgumentNamed(name) => match (ctx, Symbol::intern(name)) { + // accepted, but deprecated + (Ctx::RustcOnUnimplemented { .. }, sym::_Self) => { + warnings + .push(FormatWarning::FutureIncompat { span, help: String::from("use {Self}") }); + FormatArg::SelfUpper + } + ( + Ctx::RustcOnUnimplemented { .. }, + sym::from_desugaring + | sym::crate_local + | sym::direct + | sym::cause + | sym::float + | sym::integer_ + | sym::integral, + ) => { + warnings.push(FormatWarning::FutureIncompat { + span, + help: String::from("don't use this in a format string"), + }); + FormatArg::AsIs(String::new()) + } + + // Only `#[rustc_on_unimplemented]` can use these + (Ctx::RustcOnUnimplemented { .. }, sym::ItemContext) => FormatArg::ItemContext, + (Ctx::RustcOnUnimplemented { .. }, sym::This) => FormatArg::This, + (Ctx::RustcOnUnimplemented { .. }, sym::Trait) => FormatArg::Trait, + // `{ThisTraitsName}`. Some attrs in std use this, but I'd like to change it to the more general `{This}` + // because that'll be simpler to parse and extend in the future + (Ctx::RustcOnUnimplemented { .. }, name) if name == trait_name.name => { + warnings + .push(FormatWarning::FutureIncompat { span, help: String::from("use {This}") }); + FormatArg::This + } + + // Any attribute can use these + ( + Ctx::RustcOnUnimplemented { .. } | Ctx::DiagnosticOnUnimplemented { .. }, + kw::SelfUpper, + ) => FormatArg::SelfUpper, + ( + Ctx::RustcOnUnimplemented { .. } | Ctx::DiagnosticOnUnimplemented { .. }, + generic_param, + ) if generics.own_params.iter().any(|param| param.name == generic_param) => { + FormatArg::GenericParam { generic_param, span } + } + + (_, argument_name) => { + warnings.push(FormatWarning::UnknownParam { argument_name, span }); + FormatArg::AsIs(format!("{{{}}}", argument_name.as_str())) + } + }, + + // `{:1}` and `{}` are ignored + Position::ArgumentIs(idx) => { + warnings.push(FormatWarning::PositionalArgument { + span, + help: format!("use `{{{idx}}}` to print a number in braces"), + }); + FormatArg::AsIs(format!("{{{idx}}}")) + } + Position::ArgumentImplicitlyIs(_) => { + warnings.push(FormatWarning::PositionalArgument { + span, + help: String::from("use `{{}}` to print empty braces"), + }); + FormatArg::AsIs(String::from("{}")) + } + } +} + +/// `#[rustc_on_unimplemented]` and `#[diagnostic::...]` don't actually do anything +/// with specifiers, so emit a warning if they are used. +fn warn_on_format_spec(spec: FormatSpec<'_>, warnings: &mut Vec, input_span: Span) { + if !matches!( + spec, + FormatSpec { + fill: None, + fill_span: None, + align: Alignment::AlignUnknown, + sign: None, + alternate: false, + zero_pad: false, + debug_hex: None, + precision: Count::CountImplied, + precision_span: None, + width: Count::CountImplied, + width_span: None, + ty: _, + ty_span: _, + }, + ) { + let span = spec.ty_span.map(|inner| slice_span(input_span, inner)).unwrap_or(input_span); + warnings.push(FormatWarning::InvalidSpecifier { span, name: spec.ty.into() }) + } +} + +fn slice_span(input: Span, inner: InnerSpan) -> Span { + let InnerSpan { start, end } = inner; + let span = input.data(); + + Span::new( + span.lo + BytePos::from_usize(start), + span.lo + BytePos::from_usize(end), + span.ctxt, + span.parent, + ) +} + pub mod errors { use rustc_macros::LintDiagnostic; use rustc_middle::ty::TyCtxt; diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.stderr index 7fd51c7527f..a82a1e78da0 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.stderr +++ b/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.stderr @@ -1,52 +1,52 @@ warning: unmatched `}` found - --> $DIR/broken_format.rs:2:32 + --> $DIR/broken_format.rs:2:42 | LL | #[diagnostic::on_unimplemented(message = "{{Test } thing")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ | = note: `#[warn(unknown_or_malformed_diagnostic_attributes)]` on by default warning: positional format arguments are not allowed here - --> $DIR/broken_format.rs:7:32 + --> $DIR/broken_format.rs:7:49 | LL | #[diagnostic::on_unimplemented(message = "Test {}")] - | ^^^^^^^^^^^^^^^^^^^ + | ^ | = help: only named format arguments with the name of one of the generic types are allowed in this context warning: positional format arguments are not allowed here - --> $DIR/broken_format.rs:12:32 + --> $DIR/broken_format.rs:12:49 | LL | #[diagnostic::on_unimplemented(message = "Test {1:}")] - | ^^^^^^^^^^^^^^^^^^^^^ + | ^ | = help: only named format arguments with the name of one of the generic types are allowed in this context warning: invalid format specifier - --> $DIR/broken_format.rs:17:32 + --> $DIR/broken_format.rs:17:42 | LL | #[diagnostic::on_unimplemented(message = "Test {Self:123}")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ | = help: no format specifier are supported in this position warning: expected `}`, found `!` - --> $DIR/broken_format.rs:22:32 + --> $DIR/broken_format.rs:22:42 | LL | #[diagnostic::on_unimplemented(message = "Test {Self:!}")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ warning: unmatched `}` found - --> $DIR/broken_format.rs:22:32 + --> $DIR/broken_format.rs:22:42 | LL | #[diagnostic::on_unimplemented(message = "Test {Self:!}")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ warning: unmatched `}` found - --> $DIR/broken_format.rs:2:32 + --> $DIR/broken_format.rs:2:42 | LL | #[diagnostic::on_unimplemented(message = "{{Test } thing")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` @@ -70,10 +70,10 @@ LL | fn check_1(_: impl ImportantTrait1) {} | ^^^^^^^^^^^^^^^ required by this bound in `check_1` warning: positional format arguments are not allowed here - --> $DIR/broken_format.rs:7:32 + --> $DIR/broken_format.rs:7:49 | LL | #[diagnostic::on_unimplemented(message = "Test {}")] - | ^^^^^^^^^^^^^^^^^^^ + | ^ | = help: only named format arguments with the name of one of the generic types are allowed in this context = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` @@ -98,10 +98,10 @@ LL | fn check_2(_: impl ImportantTrait2) {} | ^^^^^^^^^^^^^^^ required by this bound in `check_2` warning: positional format arguments are not allowed here - --> $DIR/broken_format.rs:12:32 + --> $DIR/broken_format.rs:12:49 | LL | #[diagnostic::on_unimplemented(message = "Test {1:}")] - | ^^^^^^^^^^^^^^^^^^^^^ + | ^ | = help: only named format arguments with the name of one of the generic types are allowed in this context = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` @@ -126,10 +126,10 @@ LL | fn check_3(_: impl ImportantTrait3) {} | ^^^^^^^^^^^^^^^ required by this bound in `check_3` warning: invalid format specifier - --> $DIR/broken_format.rs:17:32 + --> $DIR/broken_format.rs:17:42 | LL | #[diagnostic::on_unimplemented(message = "Test {Self:123}")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ | = help: no format specifier are supported in this position = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` @@ -154,18 +154,18 @@ LL | fn check_4(_: impl ImportantTrait4) {} | ^^^^^^^^^^^^^^^ required by this bound in `check_4` warning: expected `}`, found `!` - --> $DIR/broken_format.rs:22:32 + --> $DIR/broken_format.rs:22:42 | LL | #[diagnostic::on_unimplemented(message = "Test {Self:!}")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: unmatched `}` found - --> $DIR/broken_format.rs:22:32 + --> $DIR/broken_format.rs:22:42 | LL | #[diagnostic::on_unimplemented(message = "Test {Self:!}")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.stderr index bb455d92940..88816a98dcf 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.stderr +++ b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.stderr @@ -39,82 +39,82 @@ LL | #[diagnostic::on_unimplemented = "Message"] = help: only `message`, `note` and `label` are allowed as options warning: there is no parameter `from_desugaring` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:17 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument warning: there is no parameter `direct` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:34 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument warning: there is no parameter `cause` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:42 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument warning: there is no parameter `integral` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:49 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument warning: there is no parameter `integer` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:59 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument warning: there is no parameter `float` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:15 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument warning: there is no parameter `_Self` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:22 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument warning: there is no parameter `crate_local` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:29 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument warning: there is no parameter `Trait` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:42 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument warning: there is no parameter `ItemContext` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:49 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -191,91 +191,91 @@ LL | fn takes_bar(_: impl Bar) {} | ^^^ required by this bound in `takes_bar` warning: there is no parameter `from_desugaring` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:17 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: there is no parameter `direct` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:34 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: there is no parameter `cause` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:42 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: there is no parameter `integral` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:49 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: there is no parameter `integer` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:33:59 | LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: there is no parameter `float` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:15 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: there is no parameter `_Self` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:22 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: there is no parameter `crate_local` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:29 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: there is no parameter `Trait` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:42 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: there is no parameter `ItemContext` on trait `Baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:5 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:49 | LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr index 11263580b15..4dd8c1afca0 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr +++ b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr @@ -47,10 +47,10 @@ LL | #[diagnostic::on_unimplemented] = help: at least one of the `message`, `note` and `label` options are expected warning: there is no parameter `DoesNotExist` on trait `Test` - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:32 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:44 | LL | #[diagnostic::on_unimplemented(message = "{DoesNotExist}")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -167,10 +167,10 @@ LL | fn take_whatever(_: impl Whatever) {} | ^^^^^^^^ required by this bound in `take_whatever` warning: there is no parameter `DoesNotExist` on trait `Test` - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:32 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:44 | LL | #[diagnostic::on_unimplemented(message = "{DoesNotExist}")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` diff --git a/tests/ui/on-unimplemented/bad-annotation.rs b/tests/ui/on-unimplemented/bad-annotation.rs index 3f0f69749bf..0936b25847c 100644 --- a/tests/ui/on-unimplemented/bad-annotation.rs +++ b/tests/ui/on-unimplemented/bad-annotation.rs @@ -20,12 +20,12 @@ trait BadAnnotation1 {} #[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"] -//~^ ERROR there is no parameter `C` on trait `BadAnnotation2` +//~^ WARNING there is no parameter `C` on trait `BadAnnotation2` trait BadAnnotation2 {} #[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{}>`"] -//~^ ERROR only named generic parameters are allowed +//~^ ERROR positional format arguments are not allowed here trait BadAnnotation3 {} diff --git a/tests/ui/on-unimplemented/bad-annotation.stderr b/tests/ui/on-unimplemented/bad-annotation.stderr index 4ceea779b29..1fefd93aa7e 100644 --- a/tests/ui/on-unimplemented/bad-annotation.stderr +++ b/tests/ui/on-unimplemented/bad-annotation.stderr @@ -11,17 +11,22 @@ LL | #[rustc_on_unimplemented = "message"] LL | #[rustc_on_unimplemented(/*opt*/ message = "...", /*opt*/ label = "...", /*opt*/ note = "...")] | ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ -error[E0230]: there is no parameter `C` on trait `BadAnnotation2` - --> $DIR/bad-annotation.rs:22:1 +warning: there is no parameter `C` on trait `BadAnnotation2` + --> $DIR/bad-annotation.rs:22:90 | LL | #[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ + | + = help: expect either a generic argument name or `{Self}` as format argument + = note: `#[warn(unknown_or_malformed_diagnostic_attributes)]` on by default -error[E0231]: only named generic parameters are allowed - --> $DIR/bad-annotation.rs:27:1 +warning: positional format arguments are not allowed here + --> $DIR/bad-annotation.rs:27:90 | LL | #[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{}>`"] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ + | + = help: only named format arguments with the name of one of the generic types are allowed in this context error[E0232]: this attribute must have a valid value --> $DIR/bad-annotation.rs:32:26 @@ -77,7 +82,6 @@ LL | #[rustc_on_unimplemented(on(desugared, on(desugared, message="x")), message | = note: eg `#[rustc_on_unimplemented(message="foo")]` -error: aborting due to 10 previous errors +error: aborting due to 8 previous errors; 2 warnings emitted -Some errors have detailed explanations: E0230, E0231, E0232. -For more information about an error, try `rustc --explain E0230`. +For more information about this error, try `rustc --explain E0232`. diff --git a/tests/ui/on-unimplemented/impl-substs.stderr b/tests/ui/on-unimplemented/impl-substs.stderr index b85d45eba5b..0eabe971492 100644 --- a/tests/ui/on-unimplemented/impl-substs.stderr +++ b/tests/ui/on-unimplemented/impl-substs.stderr @@ -2,7 +2,7 @@ error[E0277]: the trait bound `(i32, i32, i32): Foo` is not satisfied --> $DIR/impl-substs.rs:13:23 | LL | Foo::::foo((1i32, 1i32, 1i32)); - | ----------------- ^^^^^^^^^^^^^^^^^^ an impl did not match: usize _ _ + | ----------------- ^^^^^^^^^^^^^^^^^^ an impl did not match: usize {B} {C} | | | required by a bound introduced by this call | From ba9f51b05503a567cb637db03eaa943c59e36922 Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Thu, 27 Mar 2025 23:09:44 +0100 Subject: [PATCH 06/67] Parse condition options into a struct --- .../traits/on_unimplemented.rs | 178 ++++++++++-------- .../traits/on_unimplemented_condition.rs | 45 +++-- .../traits/on_unimplemented_format.rs | 110 ++++------- 3 files changed, 168 insertions(+), 165 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs index 6a7f3223421..7d8ecba3b2e 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs @@ -2,14 +2,13 @@ use std::iter; use std::path::PathBuf; use rustc_ast::{LitKind, MetaItem, MetaItemInner, MetaItemKind, MetaItemLit}; -use rustc_data_structures::fx::FxHashMap; use rustc_errors::codes::*; use rustc_errors::{ErrorGuaranteed, struct_span_code_err}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::{AttrArgs, Attribute}; use rustc_middle::bug; -use rustc_middle::ty::print::PrintTraitRefExt as _; -use rustc_middle::ty::{self, GenericArgsRef, GenericParamDefKind, TyCtxt}; +use rustc_middle::ty::print::PrintTraitRefExt; +use rustc_middle::ty::{self, GenericArgsRef, GenericParamDef, GenericParamDefKind, TyCtxt}; use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; use rustc_span::{Span, Symbol, sym}; use tracing::{debug, info}; @@ -17,9 +16,9 @@ use {rustc_attr_parsing as attr, rustc_hir as hir}; use super::{ObligationCauseCode, PredicateObligation}; use crate::error_reporting::TypeErrCtxt; -use crate::error_reporting::traits::on_unimplemented_condition::Condition; +use crate::error_reporting::traits::on_unimplemented_condition::{Condition, ConditionOptions}; use crate::error_reporting::traits::on_unimplemented_format::errors::*; -use crate::error_reporting::traits::on_unimplemented_format::{Ctx, FormatString}; +use crate::error_reporting::traits::on_unimplemented_format::{Ctx, FormatArgs, FormatString}; use crate::errors::{ EmptyOnClauseInOnUnimplemented, InvalidOnClauseInOnUnimplemented, NoValueInOnUnimplemented, }; @@ -107,86 +106,81 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { .unwrap_or_else(|| (trait_pred.def_id(), trait_pred.skip_binder().trait_ref.args)); let trait_pred = trait_pred.skip_binder(); - let mut flags = vec![]; + let mut self_types = vec![]; + let mut generic_args: Vec<(Symbol, String)> = vec![]; + let mut crate_local = false; // FIXME(-Zlower-impl-trait-in-trait-to-assoc-ty): HIR is not present for RPITITs, // but I guess we could synthesize one here. We don't see any errors that rely on // that yet, though. - let enclosure = self.describe_enclosure(obligation.cause.body_id).map(|t| t.to_owned()); - flags.push((sym::ItemContext, enclosure)); + let item_context = self + .describe_enclosure(obligation.cause.body_id) + .map(|t| t.to_owned()) + .unwrap_or(String::new()); - match obligation.cause.code() { + let direct = match obligation.cause.code() { ObligationCauseCode::BuiltinDerived(..) | ObligationCauseCode::ImplDerived(..) - | ObligationCauseCode::WellFormedDerived(..) => {} + | ObligationCauseCode::WellFormedDerived(..) => false, _ => { // this is a "direct", user-specified, rather than derived, // obligation. - flags.push((sym::direct, None)); + true } - } + }; - if let Some(k) = obligation.cause.span.desugaring_kind() { - flags.push((sym::from_desugaring, None)); - flags.push((sym::from_desugaring, Some(format!("{k:?}")))); - } + let from_desugaring = obligation.cause.span.desugaring_kind().map(|k| format!("{k:?}")); - if let ObligationCauseCode::MainFunctionType = obligation.cause.code() { - flags.push((sym::cause, Some("MainFunctionType".to_string()))); - } - - flags.push((sym::Trait, Some(trait_pred.trait_ref.print_trait_sugared().to_string()))); + let cause = if let ObligationCauseCode::MainFunctionType = obligation.cause.code() { + Some("MainFunctionType".to_string()) + } else { + None + }; // Add all types without trimmed paths or visible paths, ensuring they end up with // their "canonical" def path. ty::print::with_no_trimmed_paths!(ty::print::with_no_visible_paths!({ let generics = self.tcx.generics_of(def_id); let self_ty = trait_pred.self_ty(); - // This is also included through the generics list as `Self`, - // but the parser won't allow you to use it - flags.push((sym::_Self, Some(self_ty.to_string()))); + self_types.push(self_ty.to_string()); if let Some(def) = self_ty.ty_adt_def() { // We also want to be able to select self's original // signature with no type arguments resolved - flags.push(( - sym::_Self, - Some(self.tcx.type_of(def.did()).instantiate_identity().to_string()), - )); + self_types.push(self.tcx.type_of(def.did()).instantiate_identity().to_string()); } - for param in generics.own_params.iter() { - let value = match param.kind { + for GenericParamDef { name, kind, index, .. } in generics.own_params.iter() { + let value = match kind { GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { - args[param.index as usize].to_string() + args[*index as usize].to_string() } GenericParamDefKind::Lifetime => continue, }; - let name = param.name; - flags.push((name, Some(value))); + generic_args.push((*name, value)); - if let GenericParamDefKind::Type { .. } = param.kind { - let param_ty = args[param.index as usize].expect_ty(); + if let GenericParamDefKind::Type { .. } = kind { + let param_ty = args[*index as usize].expect_ty(); if let Some(def) = param_ty.ty_adt_def() { // We also want to be able to select the parameter's // original signature with no type arguments resolved - flags.push(( - name, - Some(self.tcx.type_of(def.did()).instantiate_identity().to_string()), + generic_args.push(( + *name, + self.tcx.type_of(def.did()).instantiate_identity().to_string(), )); } } } if let Some(true) = self_ty.ty_adt_def().map(|def| def.did().is_local()) { - flags.push((sym::crate_local, None)); + crate_local = true; } // Allow targeting all integers using `{integral}`, even if the exact type was resolved if self_ty.is_integral() { - flags.push((sym::_Self, Some("{integral}".to_owned()))); + self_types.push("{integral}".to_owned()); } if self_ty.is_array_slice() { - flags.push((sym::_Self, Some("&[]".to_owned()))); + self_types.push("&[]".to_owned()); } if self_ty.is_fn() { @@ -201,53 +195,51 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { hir::Safety::Unsafe => "unsafe fn", } }; - flags.push((sym::_Self, Some(shortname.to_owned()))); + self_types.push(shortname.to_owned()); } // Slices give us `[]`, `[{ty}]` if let ty::Slice(aty) = self_ty.kind() { - flags.push((sym::_Self, Some("[]".to_string()))); + self_types.push("[]".to_owned()); if let Some(def) = aty.ty_adt_def() { // We also want to be able to select the slice's type's original // signature with no type arguments resolved - flags.push(( - sym::_Self, - Some(format!("[{}]", self.tcx.type_of(def.did()).instantiate_identity())), - )); + self_types + .push(format!("[{}]", self.tcx.type_of(def.did()).instantiate_identity())); } if aty.is_integral() { - flags.push((sym::_Self, Some("[{integral}]".to_string()))); + self_types.push("[{integral}]".to_string()); } } // Arrays give us `[]`, `[{ty}; _]` and `[{ty}; N]` if let ty::Array(aty, len) = self_ty.kind() { - flags.push((sym::_Self, Some("[]".to_string()))); + self_types.push("[]".to_string()); let len = len.try_to_target_usize(self.tcx); - flags.push((sym::_Self, Some(format!("[{aty}; _]")))); + self_types.push(format!("[{aty}; _]")); if let Some(n) = len { - flags.push((sym::_Self, Some(format!("[{aty}; {n}]")))); + self_types.push(format!("[{aty}; {n}]")); } if let Some(def) = aty.ty_adt_def() { // We also want to be able to select the array's type's original // signature with no type arguments resolved let def_ty = self.tcx.type_of(def.did()).instantiate_identity(); - flags.push((sym::_Self, Some(format!("[{def_ty}; _]")))); + self_types.push(format!("[{def_ty}; _]")); if let Some(n) = len { - flags.push((sym::_Self, Some(format!("[{def_ty}; {n}]")))); + self_types.push(format!("[{def_ty}; {n}]")); } } if aty.is_integral() { - flags.push((sym::_Self, Some("[{integral}; _]".to_string()))); + self_types.push("[{integral}; _]".to_string()); if let Some(n) = len { - flags.push((sym::_Self, Some(format!("[{{integral}}; {n}]")))); + self_types.push(format!("[{{integral}}; {n}]")); } } } if let ty::Dynamic(traits, _, _) = self_ty.kind() { for t in traits.iter() { if let ty::ExistentialPredicate::Trait(trait_ref) = t.skip_binder() { - flags.push((sym::_Self, Some(self.tcx.def_path_str(trait_ref.def_id)))) + self_types.push(self.tcx.def_path_str(trait_ref.def_id)); } } } @@ -257,14 +249,51 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { && let ty::Slice(sty) = ref_ty.kind() && sty.is_integral() { - flags.push((sym::_Self, Some("&[{integral}]".to_owned()))); + self_types.push("&[{integral}]".to_owned()); } })); - flags.push((sym::This, Some(self.tcx.def_path_str(trait_pred.trait_ref.def_id)))); + let this = self.tcx.def_path_str(trait_pred.trait_ref.def_id).to_string(); + let trait_sugared = trait_pred.trait_ref.print_trait_sugared().to_string(); + + let condition_options = ConditionOptions { + self_types, + from_desugaring, + cause, + crate_local, + direct, + generic_args, + }; + + // Unlike the generic_args earlier, + // this one is *not* collected under `with_no_trimmed_paths!` + // for printing the type to the user + let generic_args = self + .tcx + .generics_of(trait_pred.trait_ref.def_id) + .own_params + .iter() + .filter_map(|param| { + let value = match param.kind { + GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { + if let Some(ty) = trait_pred.trait_ref.args[param.index as usize].as_type() + { + self.tcx.short_string(ty, long_ty_file) + } else { + trait_pred.trait_ref.args[param.index as usize].to_string() + } + } + GenericParamDefKind::Lifetime => return None, + }; + let name = param.name; + Some((name, value)) + }) + .collect(); + + let format_args = FormatArgs { this, trait_sugared, generic_args, item_context }; if let Ok(Some(command)) = OnUnimplementedDirective::of_item(self.tcx, def_id) { - command.evaluate(self.tcx, trait_pred.trait_ref, &flags, long_ty_file) + command.evaluate(self.tcx, trait_pred.trait_ref, &condition_options, &format_args) } else { OnUnimplementedNote::default() } @@ -634,23 +663,23 @@ impl<'tcx> OnUnimplementedDirective { &self, tcx: TyCtxt<'tcx>, trait_ref: ty::TraitRef<'tcx>, - options: &[(Symbol, Option)], - long_ty_file: &mut Option, + condition_options: &ConditionOptions, + args: &FormatArgs, ) -> OnUnimplementedNote { let mut message = None; let mut label = None; let mut notes = Vec::new(); let mut parent_label = None; let mut append_const_msg = None; - info!("evaluate({:?}, trait_ref={:?}, options={:?})", self, trait_ref, options); - - let options_map: FxHashMap = - options.iter().filter_map(|(k, v)| v.clone().map(|v| (*k, v))).collect(); + info!( + "evaluate({:?}, trait_ref={:?}, options={:?}, args ={:?})", + self, trait_ref, condition_options, args + ); for command in self.subcommands.iter().chain(Some(self)).rev() { debug!(?command); if let Some(ref condition) = command.condition - && !condition.matches_predicate(tcx, options, &options_map) + && !condition.matches_predicate(tcx, condition_options) { debug!("evaluate: skipping {:?} due to condition", command); continue; @@ -674,14 +703,10 @@ impl<'tcx> OnUnimplementedDirective { } OnUnimplementedNote { - label: label.map(|l| l.1.format(tcx, trait_ref, &options_map, long_ty_file)), - message: message.map(|m| m.1.format(tcx, trait_ref, &options_map, long_ty_file)), - notes: notes - .into_iter() - .map(|n| n.format(tcx, trait_ref, &options_map, long_ty_file)) - .collect(), - parent_label: parent_label - .map(|e_s| e_s.format(tcx, trait_ref, &options_map, long_ty_file)), + label: label.map(|l| l.1.format(tcx, trait_ref, args)), + message: message.map(|m| m.1.format(tcx, trait_ref, args)), + notes: notes.into_iter().map(|n| n.format(tcx, trait_ref, args)).collect(), + parent_label: parent_label.map(|e_s| e_s.format(tcx, trait_ref, args)), append_const_msg, } } @@ -759,8 +784,7 @@ impl<'tcx> OnUnimplementedFormatString { &self, tcx: TyCtxt<'tcx>, trait_ref: ty::TraitRef<'tcx>, - options: &FxHashMap, - long_ty_file: &mut Option, + args: &FormatArgs, ) -> String { let trait_def_id = trait_ref.def_id; let ctx = if self.is_diagnostic_namespace_variant { @@ -770,7 +794,7 @@ impl<'tcx> OnUnimplementedFormatString { }; if let Ok(s) = FormatString::parse(self.symbol, self.span, &ctx) { - s.format(tcx, trait_ref, options, long_ty_file) + s.format(args) } else { // we cannot return errors from processing the format string as hard error here // as the diagnostic namespace guarantees that malformed input cannot cause an error diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs index 4c852c1023a..491ba6d7ffc 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs @@ -1,9 +1,8 @@ use rustc_ast::MetaItemInner; use rustc_attr_parsing as attr; -use rustc_data_structures::fx::FxHashMap; use rustc_middle::ty::{self, TyCtxt}; use rustc_parse_format::{ParseMode, Parser, Piece, Position}; -use rustc_span::{Span, Symbol, sym}; +use rustc_span::{Span, Symbol, kw, sym}; pub static ALLOWED_CONDITION_SYMBOLS: &[Symbol] = &[ sym::from_desugaring, @@ -26,12 +25,7 @@ impl Condition { self.inner.span() } - pub fn matches_predicate<'tcx>( - &self, - tcx: TyCtxt<'tcx>, - options: &[(Symbol, Option)], - options_map: &FxHashMap, - ) -> bool { + pub fn matches_predicate<'tcx>(&self, tcx: TyCtxt<'tcx>, options: &ConditionOptions) -> bool { attr::eval_condition(&self.inner, tcx.sess, Some(tcx.features()), &mut |cfg| { let value = cfg.value.map(|v| { // `with_no_visible_paths` is also used when generating the options, @@ -44,8 +38,8 @@ impl Condition { Piece::NextArgument(a) => match a.position { Position::ArgumentNamed(arg) => { let s = Symbol::intern(arg); - match options_map.get(&s) { - Some(val) => val.to_string(), + match options.generic_args.iter().find(|(k, _)| *k == s) { + Some((_, val)) => val.to_string(), None => format!("{{{arg}}}"), } } @@ -58,7 +52,36 @@ impl Condition { }) }); - options.contains(&(cfg.name, value)) + options.contains(cfg.name, &value) }) } } + +#[derive(Debug)] +pub struct ConditionOptions { + pub self_types: Vec, + pub from_desugaring: Option, + pub cause: Option, + pub crate_local: bool, + pub direct: bool, + pub generic_args: Vec<(Symbol, String)>, +} + +impl ConditionOptions { + pub fn contains(&self, key: Symbol, value: &Option) -> bool { + match (key, value) { + (sym::_Self | kw::SelfUpper, Some(value)) => self.self_types.contains(&value), + // from_desugaring as a flag + (sym::from_desugaring, None) => self.from_desugaring.is_some(), + // from_desugaring as key == value + (sym::from_desugaring, v) => *v == self.from_desugaring, + (sym::cause, Some(value)) => self.cause.as_deref() == Some(value), + (sym::crate_local, None) => self.crate_local, + (sym::direct, None) => self.direct, + (other, Some(value)) => { + self.generic_args.iter().any(|(k, v)| *k == other && v == value) + } + _ => false, + } + } +} diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs index 21db207496c..1bf15a34509 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs @@ -1,10 +1,5 @@ -use std::fmt::Write; -use std::path::PathBuf; - use errors::*; -use rustc_data_structures::fx::FxHashMap; -use rustc_middle::span_bug; -use rustc_middle::ty::{self, GenericParamDefKind, TyCtxt}; +use rustc_middle::ty::TyCtxt; use rustc_parse_format::{ Alignment, Argument, Count, FormatSpec, InnerSpan, ParseError, ParseMode, Parser, Piece as RpfPiece, Position, @@ -13,6 +8,7 @@ use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; use rustc_span::def_id::DefId; use rustc_span::{BytePos, Pos, Span, Symbol, kw, sym}; +#[allow(dead_code)] pub struct FormatString { input: Symbol, input_span: Span, @@ -99,6 +95,24 @@ impl FormatWarning { } } +#[derive(Debug)] +pub struct ConditionOptions { + pub self_types: Vec, + pub from_desugaring: Option, + pub cause: Option, + pub crate_local: bool, + pub direct: bool, + pub generic_args: Vec<(Symbol, String)>, +} + +#[derive(Debug)] +pub struct FormatArgs { + pub this: String, + pub trait_sugared: String, + pub item_context: String, + pub generic_args: Vec<(Symbol, String)>, +} + impl FormatString { pub fn parse(input: Symbol, input_span: Span, ctx: &Ctx<'_>) -> Result> { let s = input.as_str(); @@ -126,92 +140,34 @@ impl FormatString { } } - pub fn format<'tcx>( - &self, - tcx: TyCtxt<'tcx>, - trait_ref: ty::TraitRef<'tcx>, - options: &FxHashMap, - long_ty_file: &mut Option, - ) -> String { - let generics = tcx.generics_of(trait_ref.def_id); - let generic_map = generics - .own_params - .iter() - .filter_map(|param| { - let value = match param.kind { - GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { - if let Some(ty) = trait_ref.args[param.index as usize].as_type() { - tcx.short_string(ty, long_ty_file) - } else { - trait_ref.args[param.index as usize].to_string() - } - } - GenericParamDefKind::Lifetime => return None, - }; - let name = param.name; - Some((name, value)) - }) - .collect::>(); - + pub fn format(&self, args: &FormatArgs) -> String { let mut ret = String::new(); for piece in &self.pieces { match piece { Piece::Lit(s) | Piece::Arg(FormatArg::AsIs(s)) => ret.push_str(&s), // `A` if we have `trait Trait {}` and `note = "i'm the actual type of {A}"` - Piece::Arg(FormatArg::GenericParam { generic_param, span }) => { + Piece::Arg(FormatArg::GenericParam { generic_param, .. }) => { // Should always be some but we can't raise errors here - if let Some(value) = generic_map.get(&generic_param) { - ret.push_str(value); - } else if cfg!(debug_assertions) { - span_bug!(*span, "invalid generic parameter"); - } else { - let _ = ret.write_fmt(format_args!("{{{}}}", generic_param.as_str())); - } + let value = match args.generic_args.iter().find(|(p, _)| p == generic_param) { + Some((_, val)) => val.to_string(), + None => generic_param.to_string(), + }; + ret.push_str(&value); } // `{Self}` Piece::Arg(FormatArg::SelfUpper) => { - let Some(slf) = generic_map.get(&kw::SelfUpper) else { - span_bug!( - self.input_span, - "broken format string {:?} for {:?}: \ - no argument matching `Self`", - self.input, - trait_ref, - ) + let slf = match args.generic_args.iter().find(|(p, _)| *p == kw::SelfUpper) { + Some((_, val)) => val.to_string(), + None => "Self".to_string(), }; ret.push_str(&slf); } // It's only `rustc_onunimplemented` from here - Piece::Arg(FormatArg::This) => { - let Some(this) = options.get(&sym::This) else { - span_bug!( - self.input_span, - "broken format string {:?} for {:?}: \ - no argument matching This", - self.input, - trait_ref, - ) - }; - ret.push_str(this); - } - Piece::Arg(FormatArg::Trait) => { - let Some(this) = options.get(&sym::Trait) else { - span_bug!( - self.input_span, - "broken format string {:?} for {:?}: \ - no argument matching Trait", - self.input, - trait_ref, - ) - }; - ret.push_str(this); - } - Piece::Arg(FormatArg::ItemContext) => { - let itemcontext = options.get(&sym::ItemContext); - ret.push_str(itemcontext.unwrap_or(&String::new())); - } + Piece::Arg(FormatArg::This) => ret.push_str(&args.this), + Piece::Arg(FormatArg::Trait) => ret.push_str(&args.trait_sugared), + Piece::Arg(FormatArg::ItemContext) => ret.push_str(&args.item_context), } } ret From 830bd8b6f4feb8665c8865beb20ffb424e8d8ee6 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sun, 6 Apr 2025 07:06:10 +0000 Subject: [PATCH 07/67] Implement Default for raw pointers --- library/core/src/ptr/const_ptr.rs | 8 ++++++++ library/core/src/ptr/mut_ptr.rs | 8 ++++++++ library/coretests/tests/ptr.rs | 17 +++++++++++++++++ 3 files changed, 33 insertions(+) diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 0854e31c199..2d869958b85 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -1739,3 +1739,11 @@ impl PartialOrd for *const T { *self >= *other } } + +#[stable(feature = "raw_ptr_default", since = "CURRENT_RUSTC_VERSION")] +impl Default for *const T { + /// Returns the default value of [`null()`][crate::ptr::null]. + fn default() -> Self { + crate::ptr::null() + } +} diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index e29774963db..df49eedb350 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -2156,3 +2156,11 @@ impl PartialOrd for *mut T { *self >= *other } } + +#[stable(feature = "raw_ptr_default", since = "CURRENT_RUSTC_VERSION")] +impl Default for *mut T { + /// Returns the default value of [`null_mut()`][crate::ptr::null_mut]. + fn default() -> Self { + crate::ptr::null_mut() + } +} diff --git a/library/coretests/tests/ptr.rs b/library/coretests/tests/ptr.rs index cc5f7946863..7d6e4eac1e2 100644 --- a/library/coretests/tests/ptr.rs +++ b/library/coretests/tests/ptr.rs @@ -1020,3 +1020,20 @@ fn test_ptr_swap_nonoverlapping_is_untyped() { ptr_swap_nonoverlapping_is_untyped_inner(); const { ptr_swap_nonoverlapping_is_untyped_inner() }; } + +#[test] +fn test_ptr_default() { + #[derive(Default)] + struct PtrDefaultTest { + ptr: *const u64, + } + let default = PtrDefaultTest::default(); + assert!(default.ptr.is_null()); + + #[derive(Default)] + struct PtrMutDefaultTest { + ptr: *mut u64, + } + let default = PtrMutDefaultTest::default(); + assert!(default.ptr.is_null()); +} From 8586cad77c175a0c2ce11c0579c537aa195e6da2 Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Sat, 29 Mar 2025 02:04:10 +0100 Subject: [PATCH 08/67] Documentation and finishing touches --- compiler/rustc_span/src/hygiene.rs | 19 +++ .../traits/on_unimplemented.rs | 76 ++++++++-- .../traits/on_unimplemented_condition.rs | 69 +++++++--- .../traits/on_unimplemented_format.rs | 130 +++++++++++------- 4 files changed, 215 insertions(+), 79 deletions(-) diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs index 9be16f8ce0c..f6d69da9951 100644 --- a/compiler/rustc_span/src/hygiene.rs +++ b/compiler/rustc_span/src/hygiene.rs @@ -1270,6 +1270,25 @@ impl DesugaringKind { DesugaringKind::PatTyRange => "pattern type", } } + + /// For use with `rustc_unimplemented` to support conditions + /// like `from_desugaring = "QuestionMark"` + pub fn matches(&self, value: &str) -> bool { + match self { + DesugaringKind::CondTemporary => value == "CondTemporary", + DesugaringKind::Async => value == "Async", + DesugaringKind::Await => value == "Await", + DesugaringKind::QuestionMark => value == "QuestionMark", + DesugaringKind::TryBlock => value == "TryBlock", + DesugaringKind::YeetExpr => value == "YeetExpr", + DesugaringKind::OpaqueTy => value == "OpaqueTy", + DesugaringKind::ForLoop => value == "ForLoop", + DesugaringKind::WhileLoop => value == "WhileLoop", + DesugaringKind::BoundModifier => value == "BoundModifier", + DesugaringKind::Contract => value == "Contract", + DesugaringKind::PatTyRange => value == "PatTyRange", + } + } } #[derive(Default)] diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs index 7d8ecba3b2e..0478f3a7f11 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs @@ -6,6 +6,7 @@ use rustc_errors::codes::*; use rustc_errors::{ErrorGuaranteed, struct_span_code_err}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::{AttrArgs, Attribute}; +use rustc_macros::LintDiagnostic; use rustc_middle::bug; use rustc_middle::ty::print::PrintTraitRefExt; use rustc_middle::ty::{self, GenericArgsRef, GenericParamDef, GenericParamDefKind, TyCtxt}; @@ -17,7 +18,6 @@ use {rustc_attr_parsing as attr, rustc_hir as hir}; use super::{ObligationCauseCode, PredicateObligation}; use crate::error_reporting::TypeErrCtxt; use crate::error_reporting::traits::on_unimplemented_condition::{Condition, ConditionOptions}; -use crate::error_reporting::traits::on_unimplemented_format::errors::*; use crate::error_reporting::traits::on_unimplemented_format::{Ctx, FormatArgs, FormatString}; use crate::errors::{ EmptyOnClauseInOnUnimplemented, InvalidOnClauseInOnUnimplemented, NoValueInOnUnimplemented, @@ -112,10 +112,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { // FIXME(-Zlower-impl-trait-in-trait-to-assoc-ty): HIR is not present for RPITITs, // but I guess we could synthesize one here. We don't see any errors that rely on // that yet, though. - let item_context = self - .describe_enclosure(obligation.cause.body_id) - .map(|t| t.to_owned()) - .unwrap_or(String::new()); + let item_context = self.describe_enclosure(obligation.cause.body_id).unwrap_or(""); let direct = match obligation.cause.code() { ObligationCauseCode::BuiltinDerived(..) @@ -128,7 +125,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } }; - let from_desugaring = obligation.cause.span.desugaring_kind().map(|k| format!("{k:?}")); + let from_desugaring = obligation.cause.span.desugaring_kind(); let cause = if let ObligationCauseCode::MainFunctionType = obligation.cause.code() { Some("MainFunctionType".to_string()) @@ -253,8 +250,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } })); - let this = self.tcx.def_path_str(trait_pred.trait_ref.def_id).to_string(); - let trait_sugared = trait_pred.trait_ref.print_trait_sugared().to_string(); + let this = self.tcx.def_path_str(trait_pred.trait_ref.def_id); + let trait_sugared = trait_pred.trait_ref.print_trait_sugared(); let condition_options = ConditionOptions { self_types, @@ -268,6 +265,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { // Unlike the generic_args earlier, // this one is *not* collected under `with_no_trimmed_paths!` // for printing the type to the user + // + // This includes `Self`, as it is the first parameter in `own_params`. let generic_args = self .tcx .generics_of(trait_pred.trait_ref.def_id) @@ -341,6 +340,63 @@ pub enum AppendConstMessage { Custom(Symbol, Span), } +#[derive(LintDiagnostic)] +#[diag(trait_selection_malformed_on_unimplemented_attr)] +#[help] +pub struct MalformedOnUnimplementedAttrLint { + #[label] + pub span: Span, +} + +impl MalformedOnUnimplementedAttrLint { + pub fn new(span: Span) -> Self { + Self { span } + } +} + +#[derive(LintDiagnostic)] +#[diag(trait_selection_missing_options_for_on_unimplemented_attr)] +#[help] +pub struct MissingOptionsForOnUnimplementedAttr; + +#[derive(LintDiagnostic)] +#[diag(trait_selection_ignored_diagnostic_option)] +pub struct IgnoredDiagnosticOption { + pub option_name: &'static str, + #[label] + pub span: Span, + #[label(trait_selection_other_label)] + pub prev_span: Span, +} + +impl IgnoredDiagnosticOption { + pub fn maybe_emit_warning<'tcx>( + tcx: TyCtxt<'tcx>, + item_def_id: DefId, + new: Option, + old: Option, + option_name: &'static str, + ) { + if let (Some(new_item), Some(old_item)) = (new, old) { + if let Some(item_def_id) = item_def_id.as_local() { + tcx.emit_node_span_lint( + UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, + tcx.local_def_id_to_hir_id(item_def_id), + new_item, + IgnoredDiagnosticOption { span: new_item, prev_span: old_item, option_name }, + ); + } + } + } +} + +#[derive(LintDiagnostic)] +#[diag(trait_selection_wrapped_parser_error)] +pub struct WrappedParserError { + pub description: String, + pub label: String, +} + impl<'tcx> OnUnimplementedDirective { fn parse( tcx: TyCtxt<'tcx>, @@ -664,7 +720,7 @@ impl<'tcx> OnUnimplementedDirective { tcx: TyCtxt<'tcx>, trait_ref: ty::TraitRef<'tcx>, condition_options: &ConditionOptions, - args: &FormatArgs, + args: &FormatArgs<'tcx>, ) -> OnUnimplementedNote { let mut message = None; let mut label = None; @@ -784,7 +840,7 @@ impl<'tcx> OnUnimplementedFormatString { &self, tcx: TyCtxt<'tcx>, trait_ref: ty::TraitRef<'tcx>, - args: &FormatArgs, + args: &FormatArgs<'tcx>, ) -> String { let trait_def_id = trait_ref.def_id; let ctx = if self.is_diagnostic_namespace_variant { diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs index 491ba6d7ffc..116cfb01cb6 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs @@ -2,19 +2,10 @@ use rustc_ast::MetaItemInner; use rustc_attr_parsing as attr; use rustc_middle::ty::{self, TyCtxt}; use rustc_parse_format::{ParseMode, Parser, Piece, Position}; -use rustc_span::{Span, Symbol, kw, sym}; - -pub static ALLOWED_CONDITION_SYMBOLS: &[Symbol] = &[ - sym::from_desugaring, - sym::direct, - sym::cause, - sym::integral, - sym::integer_, - sym::float, - sym::_Self, - sym::crate_local, -]; +use rustc_span::{DesugaringKind, Span, Symbol, kw, sym}; +/// A predicate in an attribute using on, all, any, +/// similar to a cfg predicate. #[derive(Debug)] pub struct Condition { pub inner: MetaItemInner, @@ -31,8 +22,7 @@ impl Condition { // `with_no_visible_paths` is also used when generating the options, // so we need to match it here. ty::print::with_no_visible_paths!({ - let mut parser = Parser::new(v.as_str(), None, None, false, ParseMode::Format); - let constructed_message = (&mut parser) + Parser::new(v.as_str(), None, None, false, ParseMode::Format) .map(|p| match p { Piece::Lit(s) => s.to_owned(), Piece::NextArgument(a) => match a.position { @@ -47,8 +37,7 @@ impl Condition { Position::ArgumentIs(idx) => format!("{{{idx}}}"), }, }) - .collect(); - constructed_message + .collect() }) }); @@ -57,13 +46,57 @@ impl Condition { } } +/// Used with `Condition::matches_predicate` to test whether the condition applies +/// +/// For example, given a +/// ```rust,ignore (just an example) +/// #[rustc_on_unimplemented( +/// on(all(from_desugaring = "QuestionMark"), +/// message = "the `?` operator can only be used in {ItemContext} \ +/// that returns `Result` or `Option` \ +/// (or another type that implements `{FromResidual}`)", +/// label = "cannot use the `?` operator in {ItemContext} that returns `{Self}`", +/// parent_label = "this function should return `Result` or `Option` to accept `?`" +/// ), +/// )] +/// pub trait FromResidual::Residual> { +/// ... +/// } +/// +/// async fn an_async_function() -> u32 { +/// let x: Option = None; +/// x?; //~ ERROR the `?` operator +/// 22 +/// } +/// ``` +/// it will look like this: +/// +/// ```rust,ignore (just an example) +/// ConditionOptions { +/// self_types: ["u32", "{integral}"], +/// from_desugaring: Some("QuestionMark"), +/// cause: None, +/// crate_local: false, +/// direct: true, +/// generic_args: [("Self","u32"), +/// ("R", "core::option::Option"), +/// ("R", "core::option::Option" ), +/// ], +/// } +/// ``` #[derive(Debug)] pub struct ConditionOptions { + /// All the self types that may apply. + /// for example pub self_types: Vec, - pub from_desugaring: Option, + // The kind of compiler desugaring. + pub from_desugaring: Option, + /// Match on a variant of [rustc_infer::traits::ObligationCauseCode] pub cause: Option, pub crate_local: bool, + /// Is the obligation "directly" user-specified, rather than derived? pub direct: bool, + // A list of the generic arguments and their reified types pub generic_args: Vec<(Symbol, String)>, } @@ -74,7 +107,7 @@ impl ConditionOptions { // from_desugaring as a flag (sym::from_desugaring, None) => self.from_desugaring.is_some(), // from_desugaring as key == value - (sym::from_desugaring, v) => *v == self.from_desugaring, + (sym::from_desugaring, Some(v)) if let Some(ds) = self.from_desugaring => ds.matches(v), (sym::cause, Some(value)) => self.cause.as_deref() == Some(value), (sym::crate_local, None) => self.crate_local, (sym::direct, None) => self.direct, diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs index 1bf15a34509..f835406122b 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs @@ -1,5 +1,8 @@ +use std::fmt; + use errors::*; use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::print::TraitRefPrintSugared; use rustc_parse_format::{ Alignment, Argument, Count, FormatSpec, InnerSpan, ParseError, ParseMode, Parser, Piece as RpfPiece, Position, @@ -8,28 +11,39 @@ use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; use rustc_span::def_id::DefId; use rustc_span::{BytePos, Pos, Span, Symbol, kw, sym}; -#[allow(dead_code)] +/// Like [std::fmt::Arguments] this is a string that has been parsed into "pieces", +/// either as string pieces or dynamic arguments. +#[derive(Debug)] pub struct FormatString { + #[allow(dead_code, reason = "Debug impl")] input: Symbol, - input_span: Span, + span: Span, pieces: Vec, - // the formatting string was parsed succesfully but with warnings + /// The formatting string was parsed succesfully but with warnings pub warnings: Vec, } +#[derive(Debug)] enum Piece { Lit(String), Arg(FormatArg), } -pub enum FormatArg { +#[derive(Debug)] +enum FormatArg { // A generic parameter, like `{T}` if we're on the `From` trait. - GenericParam { generic_param: Symbol, span: Span }, + GenericParam { + generic_param: Symbol, + }, // `{Self}` SelfUpper, + /// `{This}` or `{TraitName}` This, + /// The sugared form of the trait Trait, + /// what we're in, like a function, method, closure etc. ItemContext, + /// What the user typed, if it doesn't match anything we can use. AsIs(String), } @@ -40,6 +54,7 @@ pub enum Ctx<'tcx> { DiagnosticOnUnimplemented { tcx: TyCtxt<'tcx>, trait_def_id: DefId }, } +#[derive(Debug)] pub enum FormatWarning { UnknownParam { argument_name: Symbol, span: Span }, PositionalArgument { span: Span, help: String }, @@ -95,26 +110,58 @@ impl FormatWarning { } } +/// Arguments to fill a [FormatString] with. +/// +/// For example, given a +/// ```rust,ignore (just an example) +/// +/// #[rustc_on_unimplemented( +/// on(all(from_desugaring = "QuestionMark"), +/// message = "the `?` operator can only be used in {ItemContext} \ +/// that returns `Result` or `Option` \ +/// (or another type that implements `{FromResidual}`)", +/// label = "cannot use the `?` operator in {ItemContext} that returns `{Self}`", +/// parent_label = "this function should return `Result` or `Option` to accept `?`" +/// ), +/// )] +/// pub trait FromResidual::Residual> { +/// ... +/// } +/// +/// async fn an_async_function() -> u32 { +/// let x: Option = None; +/// x?; //~ ERROR the `?` operator +/// 22 +/// } +/// ``` +/// it will look like this: +/// +/// ```rust,ignore (just an example) +/// FormatArgs { +/// this: "FromResidual", +/// trait_sugared: "FromResidual>", +/// item_context: "an async function", +/// generic_args: [("Self", "u32"), ("R", "Option")], +/// } +/// ``` #[derive(Debug)] -pub struct ConditionOptions { - pub self_types: Vec, - pub from_desugaring: Option, - pub cause: Option, - pub crate_local: bool, - pub direct: bool, - pub generic_args: Vec<(Symbol, String)>, -} - -#[derive(Debug)] -pub struct FormatArgs { +pub struct FormatArgs<'tcx> { pub this: String, - pub trait_sugared: String, - pub item_context: String, + pub trait_sugared: TraitRefPrintSugared<'tcx>, + pub item_context: &'static str, pub generic_args: Vec<(Symbol, String)>, } impl FormatString { - pub fn parse(input: Symbol, input_span: Span, ctx: &Ctx<'_>) -> Result> { + pub fn span(&self) -> Span { + self.span + } + + pub fn parse<'tcx>( + input: Symbol, + span: Span, + ctx: &Ctx<'tcx>, + ) -> Result> { let s = input.as_str(); let mut parser = Parser::new(s, None, None, false, ParseMode::Format); let mut pieces = Vec::new(); @@ -126,28 +173,28 @@ impl FormatString { pieces.push(Piece::Lit(lit.into())); } RpfPiece::NextArgument(arg) => { - warn_on_format_spec(arg.format, &mut warnings, input_span); - let arg = parse_arg(&arg, ctx, &mut warnings, input_span); + warn_on_format_spec(arg.format, &mut warnings, span); + let arg = parse_arg(&arg, ctx, &mut warnings, span); pieces.push(Piece::Arg(arg)); } } } if parser.errors.is_empty() { - Ok(FormatString { input, input_span, pieces, warnings }) + Ok(FormatString { input, pieces, span, warnings }) } else { Err(parser.errors) } } - pub fn format(&self, args: &FormatArgs) -> String { + pub fn format(&self, args: &FormatArgs<'_>) -> String { let mut ret = String::new(); for piece in &self.pieces { match piece { Piece::Lit(s) | Piece::Arg(FormatArg::AsIs(s)) => ret.push_str(&s), // `A` if we have `trait Trait {}` and `note = "i'm the actual type of {A}"` - Piece::Arg(FormatArg::GenericParam { generic_param, .. }) => { + Piece::Arg(FormatArg::GenericParam { generic_param }) => { // Should always be some but we can't raise errors here let value = match args.generic_args.iter().find(|(p, _)| p == generic_param) { Some((_, val)) => val.to_string(), @@ -166,17 +213,19 @@ impl FormatString { // It's only `rustc_onunimplemented` from here Piece::Arg(FormatArg::This) => ret.push_str(&args.this), - Piece::Arg(FormatArg::Trait) => ret.push_str(&args.trait_sugared), - Piece::Arg(FormatArg::ItemContext) => ret.push_str(&args.item_context), + Piece::Arg(FormatArg::Trait) => { + let _ = fmt::write(&mut ret, format_args!("{}", &args.trait_sugared)); + } + Piece::Arg(FormatArg::ItemContext) => ret.push_str(args.item_context), } } ret } } -fn parse_arg( +fn parse_arg<'tcx>( arg: &Argument<'_>, - ctx: &Ctx<'_>, + ctx: &Ctx<'tcx>, warnings: &mut Vec, input_span: Span, ) -> FormatArg { @@ -233,7 +282,7 @@ fn parse_arg( Ctx::RustcOnUnimplemented { .. } | Ctx::DiagnosticOnUnimplemented { .. }, generic_param, ) if generics.own_params.iter().any(|param| param.name == generic_param) => { - FormatArg::GenericParam { generic_param, span } + FormatArg::GenericParam { generic_param } } (_, argument_name) => { @@ -286,6 +335,7 @@ fn warn_on_format_spec(spec: FormatSpec<'_>, warnings: &mut Vec, } } +/// Helper function because `Span` and `rustc_parse_format::InnerSpan` don't know about each other fn slice_span(input: Span, inner: InnerSpan) -> Span { let InnerSpan { start, end } = inner; let span = input.data(); @@ -300,8 +350,6 @@ fn slice_span(input: Span, inner: InnerSpan) -> Span { pub mod errors { use rustc_macros::LintDiagnostic; - use rustc_middle::ty::TyCtxt; - use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; use rustc_span::Ident; use super::*; @@ -324,26 +372,6 @@ pub mod errors { #[help] pub struct InvalidFormatSpecifier; - #[derive(LintDiagnostic)] - #[diag(trait_selection_wrapped_parser_error)] - pub struct WrappedParserError { - pub description: String, - pub label: String, - } - #[derive(LintDiagnostic)] - #[diag(trait_selection_malformed_on_unimplemented_attr)] - #[help] - pub struct MalformedOnUnimplementedAttrLint { - #[label] - pub span: Span, - } - - impl MalformedOnUnimplementedAttrLint { - pub fn new(span: Span) -> Self { - Self { span } - } - } - #[derive(LintDiagnostic)] #[diag(trait_selection_missing_options_for_on_unimplemented_attr)] #[help] From 9abaa9d4dffeb897a1dbff97d32d3b6ac190be21 Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Sat, 29 Mar 2025 11:46:03 +0100 Subject: [PATCH 09/67] Disable usage on trait impls and aliases --- .../traits/on_unimplemented.rs | 17 +++++++--- tests/crashes/130627.rs | 20 ------------ .../ui/diagnostic_namespace/on_impl_trait.rs | 17 ++++++++++ .../diagnostic_namespace/on_impl_trait.stderr | 31 +++++++++++++++++++ tests/ui/on-unimplemented/impl-substs.stderr | 2 +- .../ui/on-unimplemented/multiple-impls.stderr | 12 +++---- tests/ui/on-unimplemented/on-impl.stderr | 4 +-- 7 files changed, 68 insertions(+), 35 deletions(-) delete mode 100644 tests/crashes/130627.rs create mode 100644 tests/ui/diagnostic_namespace/on_impl_trait.rs create mode 100644 tests/ui/diagnostic_namespace/on_impl_trait.stderr diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs index 0478f3a7f11..ca51f177f96 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs @@ -553,6 +553,13 @@ impl<'tcx> OnUnimplementedDirective { } pub fn of_item(tcx: TyCtxt<'tcx>, item_def_id: DefId) -> Result, ErrorGuaranteed> { + if !tcx.is_trait(item_def_id) { + // It could be a trait_alias (`trait MyTrait = SomeOtherTrait`) + // or an implementation (`impl MyTrait for Foo {}`) + // + // We don't support those. + return Ok(None); + } if let Some(attr) = tcx.get_attr(item_def_id, sym::rustc_on_unimplemented) { return Self::parse_attribute(attr, false, tcx, item_def_id); } else { @@ -782,8 +789,10 @@ impl<'tcx> OnUnimplementedFormatString { Ok(result) } - fn verify(&self, tcx: TyCtxt<'tcx>, item_def_id: DefId) -> Result<(), ErrorGuaranteed> { - let trait_def_id = if tcx.is_trait(item_def_id) { item_def_id } else { return Ok(()) }; + fn verify(&self, tcx: TyCtxt<'tcx>, trait_def_id: DefId) -> Result<(), ErrorGuaranteed> { + if !tcx.is_trait(trait_def_id) { + return Ok(()); + }; let ctx = if self.is_diagnostic_namespace_variant { Ctx::DiagnosticOnUnimplemented { tcx, trait_def_id } @@ -810,10 +819,10 @@ impl<'tcx> OnUnimplementedFormatString { // so that users are aware that something is not correct for e in errors { if self.is_diagnostic_namespace_variant { - if let Some(item_def_id) = item_def_id.as_local() { + if let Some(trait_def_id) = trait_def_id.as_local() { tcx.emit_node_span_lint( UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, - tcx.local_def_id_to_hir_id(item_def_id), + tcx.local_def_id_to_hir_id(trait_def_id), self.span, WrappedParserError { description: e.description, label: e.label }, ); diff --git a/tests/crashes/130627.rs b/tests/crashes/130627.rs deleted file mode 100644 index 59d3606592b..00000000000 --- a/tests/crashes/130627.rs +++ /dev/null @@ -1,20 +0,0 @@ -//@ known-bug: #130627 - -#![feature(trait_alias)] - -trait Test {} - -#[diagnostic::on_unimplemented( - message="message", - label="label", - note="note" -)] -trait Alias = Test; - -// Use trait alias as bound on type parameter. -fn foo(v: &T) { -} - -pub fn main() { - foo(&1); -} diff --git a/tests/ui/diagnostic_namespace/on_impl_trait.rs b/tests/ui/diagnostic_namespace/on_impl_trait.rs new file mode 100644 index 00000000000..32a492c53a9 --- /dev/null +++ b/tests/ui/diagnostic_namespace/on_impl_trait.rs @@ -0,0 +1,17 @@ +// used to ICE, see +// Instead it should just ignore the diagnostic attribute +#![feature(trait_alias)] + +trait Test {} + +#[diagnostic::on_unimplemented(message = "blah", label = "blah", note = "blah")] +//~^ WARN `#[diagnostic::on_unimplemented]` can only be applied to trait definitions +trait Alias = Test; + +// Use trait alias as bound on type parameter. +fn foo(v: &T) {} + +pub fn main() { + foo(&1); + //~^ ERROR the trait bound `{integer}: Alias` is not satisfied +} diff --git a/tests/ui/diagnostic_namespace/on_impl_trait.stderr b/tests/ui/diagnostic_namespace/on_impl_trait.stderr new file mode 100644 index 00000000000..59b9c31bc53 --- /dev/null +++ b/tests/ui/diagnostic_namespace/on_impl_trait.stderr @@ -0,0 +1,31 @@ +warning: `#[diagnostic::on_unimplemented]` can only be applied to trait definitions + --> $DIR/on_impl_trait.rs:7:1 + | +LL | #[diagnostic::on_unimplemented(message = "blah", label = "blah", note = "blah")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(unknown_or_malformed_diagnostic_attributes)]` on by default + +error[E0277]: the trait bound `{integer}: Alias` is not satisfied + --> $DIR/on_impl_trait.rs:15:9 + | +LL | foo(&1); + | --- ^^ the trait `Test` is not implemented for `{integer}` + | | + | required by a bound introduced by this call + | +help: this trait has no implementations, consider adding one + --> $DIR/on_impl_trait.rs:5:1 + | +LL | trait Test {} + | ^^^^^^^^^^ + = note: required for `{integer}` to implement `Alias` +note: required by a bound in `foo` + --> $DIR/on_impl_trait.rs:12:11 + | +LL | fn foo(v: &T) {} + | ^^^^^ required by this bound in `foo` + +error: aborting due to 1 previous error; 1 warning emitted + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/on-unimplemented/impl-substs.stderr b/tests/ui/on-unimplemented/impl-substs.stderr index 0eabe971492..2d83845ecb8 100644 --- a/tests/ui/on-unimplemented/impl-substs.stderr +++ b/tests/ui/on-unimplemented/impl-substs.stderr @@ -2,7 +2,7 @@ error[E0277]: the trait bound `(i32, i32, i32): Foo` is not satisfied --> $DIR/impl-substs.rs:13:23 | LL | Foo::::foo((1i32, 1i32, 1i32)); - | ----------------- ^^^^^^^^^^^^^^^^^^ an impl did not match: usize {B} {C} + | ----------------- ^^^^^^^^^^^^^^^^^^ the trait `Foo` is not implemented for `(i32, i32, i32)` | | | required by a bound introduced by this call | diff --git a/tests/ui/on-unimplemented/multiple-impls.stderr b/tests/ui/on-unimplemented/multiple-impls.stderr index ba4e43ff359..2afc9b1bf3b 100644 --- a/tests/ui/on-unimplemented/multiple-impls.stderr +++ b/tests/ui/on-unimplemented/multiple-impls.stderr @@ -15,11 +15,10 @@ error[E0277]: the trait bound `[i32]: Index>` is not satisfied --> $DIR/multiple-impls.rs:36:33 | LL | Index::index(&[] as &[i32], Foo(2u32)); - | ------------ ^^^^^^^^^ on impl for Foo + | ------------ ^^^^^^^^^ the trait `Index>` is not implemented for `[i32]` | | | required by a bound introduced by this call | - = help: the trait `Index>` is not implemented for `[i32]` = help: the following other types implement trait `Index`: `[i32]` implements `Index>` `[i32]` implements `Index>` @@ -28,11 +27,10 @@ error[E0277]: the trait bound `[i32]: Index>` is not satisfied --> $DIR/multiple-impls.rs:39:33 | LL | Index::index(&[] as &[i32], Bar(2u32)); - | ------------ ^^^^^^^^^ on impl for Bar + | ------------ ^^^^^^^^^ the trait `Index>` is not implemented for `[i32]` | | | required by a bound introduced by this call | - = help: the trait `Index>` is not implemented for `[i32]` = help: the following other types implement trait `Index`: `[i32]` implements `Index>` `[i32]` implements `Index>` @@ -52,9 +50,8 @@ error[E0277]: the trait bound `[i32]: Index>` is not satisfied --> $DIR/multiple-impls.rs:36:5 | LL | Index::index(&[] as &[i32], Foo(2u32)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ on impl for Foo + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Index>` is not implemented for `[i32]` | - = help: the trait `Index>` is not implemented for `[i32]` = help: the following other types implement trait `Index`: `[i32]` implements `Index>` `[i32]` implements `Index>` @@ -63,9 +60,8 @@ error[E0277]: the trait bound `[i32]: Index>` is not satisfied --> $DIR/multiple-impls.rs:39:5 | LL | Index::index(&[] as &[i32], Bar(2u32)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ on impl for Bar + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Index>` is not implemented for `[i32]` | - = help: the trait `Index>` is not implemented for `[i32]` = help: the following other types implement trait `Index`: `[i32]` implements `Index>` `[i32]` implements `Index>` diff --git a/tests/ui/on-unimplemented/on-impl.stderr b/tests/ui/on-unimplemented/on-impl.stderr index 5e7e2c4ea77..922db9db116 100644 --- a/tests/ui/on-unimplemented/on-impl.stderr +++ b/tests/ui/on-unimplemented/on-impl.stderr @@ -2,7 +2,7 @@ error[E0277]: the trait bound `[i32]: Index` is not satisfied --> $DIR/on-impl.rs:22:47 | LL | Index::::index(&[1, 2, 3] as &[i32], 2u32); - | ------------------- ^^^^ a usize is required to index into a slice + | ------------------- ^^^^ the trait `Index` is not implemented for `[i32]` | | | required by a bound introduced by this call | @@ -14,7 +14,7 @@ error[E0277]: the trait bound `[i32]: Index` is not satisfied --> $DIR/on-impl.rs:22:5 | LL | Index::::index(&[1, 2, 3] as &[i32], 2u32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a usize is required to index into a slice + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Index` is not implemented for `[i32]` | = help: the trait `Index` is not implemented for `[i32]` but trait `Index` is implemented for it From 10ec5cbe96b8b1ba96fbacc207d6a2f0d19ca9e2 Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Sat, 29 Mar 2025 13:48:14 +0100 Subject: [PATCH 10/67] Raise errors on bad rustc_on_unimplemented format strings again --- .../traits/on_unimplemented.rs | 38 +++++++++++++++++-- tests/ui/on-unimplemented/bad-annotation.rs | 2 +- .../ui/on-unimplemented/bad-annotation.stderr | 14 +++---- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs index ca51f177f96..4c4491269b7 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs @@ -18,7 +18,9 @@ use {rustc_attr_parsing as attr, rustc_hir as hir}; use super::{ObligationCauseCode, PredicateObligation}; use crate::error_reporting::TypeErrCtxt; use crate::error_reporting::traits::on_unimplemented_condition::{Condition, ConditionOptions}; -use crate::error_reporting::traits::on_unimplemented_format::{Ctx, FormatArgs, FormatString}; +use crate::error_reporting::traits::on_unimplemented_format::{ + Ctx, FormatArgs, FormatString, FormatWarning, +}; use crate::errors::{ EmptyOnClauseInOnUnimplemented, InvalidOnClauseInOnUnimplemented, NoValueInOnUnimplemented, }; @@ -806,8 +808,38 @@ impl<'tcx> OnUnimplementedFormatString { // Warnings about format specifiers, deprecated parameters, wrong parameters etc. // In other words we'd like to let the author know, but we can still try to format the string later Ok(FormatString { warnings, .. }) => { - for w in warnings { - w.emit_warning(tcx, trait_def_id) + if self.is_diagnostic_namespace_variant { + for w in warnings { + w.emit_warning(tcx, trait_def_id) + } + } else { + for w in warnings { + match w { + FormatWarning::UnknownParam { argument_name, span } => { + let reported = struct_span_code_err!( + tcx.dcx(), + span, + E0230, + "cannot find parameter {} on this trait", + argument_name, + ) + .emit(); + result = Err(reported); + } + FormatWarning::PositionalArgument { span, .. } => { + let reported = struct_span_code_err!( + tcx.dcx(), + span, + E0231, + "positional format arguments are not allowed here" + ) + .emit(); + result = Err(reported); + } + FormatWarning::InvalidSpecifier { .. } + | FormatWarning::FutureIncompat { .. } => {} + } + } } } // Errors from the underlying `rustc_parse_format::Parser` diff --git a/tests/ui/on-unimplemented/bad-annotation.rs b/tests/ui/on-unimplemented/bad-annotation.rs index 0936b25847c..f2b97865759 100644 --- a/tests/ui/on-unimplemented/bad-annotation.rs +++ b/tests/ui/on-unimplemented/bad-annotation.rs @@ -20,7 +20,7 @@ trait BadAnnotation1 {} #[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"] -//~^ WARNING there is no parameter `C` on trait `BadAnnotation2` +//~^ ERROR cannot find parameter C on this trait trait BadAnnotation2 {} diff --git a/tests/ui/on-unimplemented/bad-annotation.stderr b/tests/ui/on-unimplemented/bad-annotation.stderr index 1fefd93aa7e..afd737dc85e 100644 --- a/tests/ui/on-unimplemented/bad-annotation.stderr +++ b/tests/ui/on-unimplemented/bad-annotation.stderr @@ -11,22 +11,17 @@ LL | #[rustc_on_unimplemented = "message"] LL | #[rustc_on_unimplemented(/*opt*/ message = "...", /*opt*/ label = "...", /*opt*/ note = "...")] | ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ -warning: there is no parameter `C` on trait `BadAnnotation2` +error[E0230]: cannot find parameter C on this trait --> $DIR/bad-annotation.rs:22:90 | LL | #[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"] | ^ - | - = help: expect either a generic argument name or `{Self}` as format argument - = note: `#[warn(unknown_or_malformed_diagnostic_attributes)]` on by default -warning: positional format arguments are not allowed here +error[E0231]: positional format arguments are not allowed here --> $DIR/bad-annotation.rs:27:90 | LL | #[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{}>`"] | ^ - | - = help: only named format arguments with the name of one of the generic types are allowed in this context error[E0232]: this attribute must have a valid value --> $DIR/bad-annotation.rs:32:26 @@ -82,6 +77,7 @@ LL | #[rustc_on_unimplemented(on(desugared, on(desugared, message="x")), message | = note: eg `#[rustc_on_unimplemented(message="foo")]` -error: aborting due to 8 previous errors; 2 warnings emitted +error: aborting due to 10 previous errors -For more information about this error, try `rustc --explain E0232`. +Some errors have detailed explanations: E0230, E0231, E0232. +For more information about an error, try `rustc --explain E0230`. From 40e76c157548442fce755ec9cc2274aef11454ba Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Tue, 1 Apr 2025 02:47:12 +0200 Subject: [PATCH 11/67] Test that `Self` properly works in filters --- .../ui/on-unimplemented/use_self_no_underscore.rs | 14 ++++++++++++++ .../use_self_no_underscore.stderr | 15 +++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 tests/ui/on-unimplemented/use_self_no_underscore.rs create mode 100644 tests/ui/on-unimplemented/use_self_no_underscore.stderr diff --git a/tests/ui/on-unimplemented/use_self_no_underscore.rs b/tests/ui/on-unimplemented/use_self_no_underscore.rs new file mode 100644 index 00000000000..045ef1a5d3f --- /dev/null +++ b/tests/ui/on-unimplemented/use_self_no_underscore.rs @@ -0,0 +1,14 @@ +#![feature(rustc_attrs)] + +#[rustc_on_unimplemented(on( + all(A = "{integer}", any(Self = "[{integral}; _]",)), + message = "an array of type `{Self}` cannot be built directly from an iterator", +))] +pub trait FromIterator: Sized { + fn from_iter>(iter: T) -> Self; +} +fn main() { + let iter = 0..42_8; + let x: [u8; 8] = FromIterator::from_iter(iter); + //~^ ERROR an array of type `[u8; 8]` cannot be built directly from an iterator +} diff --git a/tests/ui/on-unimplemented/use_self_no_underscore.stderr b/tests/ui/on-unimplemented/use_self_no_underscore.stderr new file mode 100644 index 00000000000..d01aee3485f --- /dev/null +++ b/tests/ui/on-unimplemented/use_self_no_underscore.stderr @@ -0,0 +1,15 @@ +error[E0277]: an array of type `[u8; 8]` cannot be built directly from an iterator + --> $DIR/use_self_no_underscore.rs:12:22 + | +LL | let x: [u8; 8] = FromIterator::from_iter(iter); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `FromIterator<{integer}>` is not implemented for `[u8; 8]` + | +help: this trait has no implementations, consider adding one + --> $DIR/use_self_no_underscore.rs:7:1 + | +LL | pub trait FromIterator: Sized { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. From 4a5369b476fb17aa2db05b5b551eca0cfa6b2c6b Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Thu, 10 Apr 2025 16:06:59 +0200 Subject: [PATCH 12/67] Remove rustc_on_unimplemented on impl tests --- src/tools/tidy/src/issues.txt | 1 - tests/ui/on-unimplemented/impl-substs.rs | 15 ---- tests/ui/on-unimplemented/impl-substs.stderr | 15 ---- tests/ui/on-unimplemented/issue-104140.rs | 8 --- tests/ui/on-unimplemented/issue-104140.stderr | 15 ---- tests/ui/on-unimplemented/multiple-impls.rs | 42 ----------- .../ui/on-unimplemented/multiple-impls.stderr | 71 ------------------- tests/ui/on-unimplemented/on-impl.rs | 25 ------- tests/ui/on-unimplemented/on-impl.stderr | 25 ------- 9 files changed, 217 deletions(-) delete mode 100644 tests/ui/on-unimplemented/impl-substs.rs delete mode 100644 tests/ui/on-unimplemented/impl-substs.stderr delete mode 100644 tests/ui/on-unimplemented/issue-104140.rs delete mode 100644 tests/ui/on-unimplemented/issue-104140.stderr delete mode 100644 tests/ui/on-unimplemented/multiple-impls.rs delete mode 100644 tests/ui/on-unimplemented/multiple-impls.stderr delete mode 100644 tests/ui/on-unimplemented/on-impl.rs delete mode 100644 tests/ui/on-unimplemented/on-impl.stderr diff --git a/src/tools/tidy/src/issues.txt b/src/tools/tidy/src/issues.txt index d2ae9b1f6ef..5fa4a9fc2e3 100644 --- a/src/tools/tidy/src/issues.txt +++ b/src/tools/tidy/src/issues.txt @@ -3138,7 +3138,6 @@ ui/nll/user-annotations/issue-55241.rs ui/nll/user-annotations/issue-55748-pat-types-constrain-bindings.rs ui/nll/user-annotations/issue-57731-ascibed-coupled-types.rs ui/numbers-arithmetic/issue-8460.rs -ui/on-unimplemented/issue-104140.rs ui/or-patterns/issue-64879-trailing-before-guard.rs ui/or-patterns/issue-67514-irrefutable-param.rs ui/or-patterns/issue-68785-irrefutable-param-with-at.rs diff --git a/tests/ui/on-unimplemented/impl-substs.rs b/tests/ui/on-unimplemented/impl-substs.rs deleted file mode 100644 index fe9c50ec3d4..00000000000 --- a/tests/ui/on-unimplemented/impl-substs.rs +++ /dev/null @@ -1,15 +0,0 @@ -#![feature(rustc_attrs)] - -trait Foo { - fn foo(self); -} - -#[rustc_on_unimplemented = "an impl did not match: {A} {B} {C}"] -impl Foo for (A, B, C) { - fn foo(self) {} -} - -fn main() { - Foo::::foo((1i32, 1i32, 1i32)); - //~^ ERROR the trait bound `(i32, i32, i32): Foo` is not satisfied -} diff --git a/tests/ui/on-unimplemented/impl-substs.stderr b/tests/ui/on-unimplemented/impl-substs.stderr deleted file mode 100644 index 2d83845ecb8..00000000000 --- a/tests/ui/on-unimplemented/impl-substs.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0277]: the trait bound `(i32, i32, i32): Foo` is not satisfied - --> $DIR/impl-substs.rs:13:23 - | -LL | Foo::::foo((1i32, 1i32, 1i32)); - | ----------------- ^^^^^^^^^^^^^^^^^^ the trait `Foo` is not implemented for `(i32, i32, i32)` - | | - | required by a bound introduced by this call - | - = help: the trait `Foo` is not implemented for `(i32, i32, i32)` - but trait `Foo` is implemented for it - = help: for that trait implementation, expected `i32`, found `usize` - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/on-unimplemented/issue-104140.rs b/tests/ui/on-unimplemented/issue-104140.rs deleted file mode 100644 index ade3f727004..00000000000 --- a/tests/ui/on-unimplemented/issue-104140.rs +++ /dev/null @@ -1,8 +0,0 @@ -#![feature(rustc_attrs)] - -trait Foo {} - -#[rustc_on_unimplemented] //~ ERROR malformed `rustc_on_unimplemented` attribute input -impl Foo for u32 {} - -fn main() {} diff --git a/tests/ui/on-unimplemented/issue-104140.stderr b/tests/ui/on-unimplemented/issue-104140.stderr deleted file mode 100644 index 3c317135dd4..00000000000 --- a/tests/ui/on-unimplemented/issue-104140.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error: malformed `rustc_on_unimplemented` attribute input - --> $DIR/issue-104140.rs:5:1 - | -LL | #[rustc_on_unimplemented] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: the following are the possible correct uses - | -LL | #[rustc_on_unimplemented = "message"] - | +++++++++++ -LL | #[rustc_on_unimplemented(/*opt*/ message = "...", /*opt*/ label = "...", /*opt*/ note = "...")] - | ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ - -error: aborting due to 1 previous error - diff --git a/tests/ui/on-unimplemented/multiple-impls.rs b/tests/ui/on-unimplemented/multiple-impls.rs deleted file mode 100644 index b74957ebcd4..00000000000 --- a/tests/ui/on-unimplemented/multiple-impls.rs +++ /dev/null @@ -1,42 +0,0 @@ -// Test if the on_unimplemented message override works - -#![feature(rustc_attrs)] - - -struct Foo(T); -struct Bar(T); - -#[rustc_on_unimplemented = "trait message"] -trait Index { - type Output: ?Sized; - fn index(&self, index: Idx) -> &Self::Output; -} - -#[rustc_on_unimplemented = "on impl for Foo"] -impl Index> for [i32] { - type Output = i32; - fn index(&self, _index: Foo) -> &i32 { - loop {} - } -} - -#[rustc_on_unimplemented = "on impl for Bar"] -impl Index> for [i32] { - type Output = i32; - fn index(&self, _index: Bar) -> &i32 { - loop {} - } -} - - -fn main() { - Index::index(&[] as &[i32], 2u32); - //~^ ERROR E0277 - //~| ERROR E0277 - Index::index(&[] as &[i32], Foo(2u32)); - //~^ ERROR E0277 - //~| ERROR E0277 - Index::index(&[] as &[i32], Bar(2u32)); - //~^ ERROR E0277 - //~| ERROR E0277 -} diff --git a/tests/ui/on-unimplemented/multiple-impls.stderr b/tests/ui/on-unimplemented/multiple-impls.stderr deleted file mode 100644 index 2afc9b1bf3b..00000000000 --- a/tests/ui/on-unimplemented/multiple-impls.stderr +++ /dev/null @@ -1,71 +0,0 @@ -error[E0277]: the trait bound `[i32]: Index` is not satisfied - --> $DIR/multiple-impls.rs:33:33 - | -LL | Index::index(&[] as &[i32], 2u32); - | ------------ ^^^^ trait message - | | - | required by a bound introduced by this call - | - = help: the trait `Index` is not implemented for `[i32]` - = help: the following other types implement trait `Index`: - `[i32]` implements `Index>` - `[i32]` implements `Index>` - -error[E0277]: the trait bound `[i32]: Index>` is not satisfied - --> $DIR/multiple-impls.rs:36:33 - | -LL | Index::index(&[] as &[i32], Foo(2u32)); - | ------------ ^^^^^^^^^ the trait `Index>` is not implemented for `[i32]` - | | - | required by a bound introduced by this call - | - = help: the following other types implement trait `Index`: - `[i32]` implements `Index>` - `[i32]` implements `Index>` - -error[E0277]: the trait bound `[i32]: Index>` is not satisfied - --> $DIR/multiple-impls.rs:39:33 - | -LL | Index::index(&[] as &[i32], Bar(2u32)); - | ------------ ^^^^^^^^^ the trait `Index>` is not implemented for `[i32]` - | | - | required by a bound introduced by this call - | - = help: the following other types implement trait `Index`: - `[i32]` implements `Index>` - `[i32]` implements `Index>` - -error[E0277]: the trait bound `[i32]: Index` is not satisfied - --> $DIR/multiple-impls.rs:33:5 - | -LL | Index::index(&[] as &[i32], 2u32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trait message - | - = help: the trait `Index` is not implemented for `[i32]` - = help: the following other types implement trait `Index`: - `[i32]` implements `Index>` - `[i32]` implements `Index>` - -error[E0277]: the trait bound `[i32]: Index>` is not satisfied - --> $DIR/multiple-impls.rs:36:5 - | -LL | Index::index(&[] as &[i32], Foo(2u32)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Index>` is not implemented for `[i32]` - | - = help: the following other types implement trait `Index`: - `[i32]` implements `Index>` - `[i32]` implements `Index>` - -error[E0277]: the trait bound `[i32]: Index>` is not satisfied - --> $DIR/multiple-impls.rs:39:5 - | -LL | Index::index(&[] as &[i32], Bar(2u32)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Index>` is not implemented for `[i32]` - | - = help: the following other types implement trait `Index`: - `[i32]` implements `Index>` - `[i32]` implements `Index>` - -error: aborting due to 6 previous errors - -For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/on-unimplemented/on-impl.rs b/tests/ui/on-unimplemented/on-impl.rs deleted file mode 100644 index ab3e67d01fe..00000000000 --- a/tests/ui/on-unimplemented/on-impl.rs +++ /dev/null @@ -1,25 +0,0 @@ -// Test if the on_unimplemented message override works - -#![feature(rustc_attrs)] - - -#[rustc_on_unimplemented = "invalid"] -trait Index { - type Output: ?Sized; - fn index(&self, index: Idx) -> &Self::Output; -} - -#[rustc_on_unimplemented = "a usize is required to index into a slice"] -impl Index for [i32] { - type Output = i32; - fn index(&self, index: usize) -> &i32 { - &self[index] - } -} - - -fn main() { - Index::::index(&[1, 2, 3] as &[i32], 2u32); - //~^ ERROR E0277 - //~| ERROR E0277 -} diff --git a/tests/ui/on-unimplemented/on-impl.stderr b/tests/ui/on-unimplemented/on-impl.stderr deleted file mode 100644 index 922db9db116..00000000000 --- a/tests/ui/on-unimplemented/on-impl.stderr +++ /dev/null @@ -1,25 +0,0 @@ -error[E0277]: the trait bound `[i32]: Index` is not satisfied - --> $DIR/on-impl.rs:22:47 - | -LL | Index::::index(&[1, 2, 3] as &[i32], 2u32); - | ------------------- ^^^^ the trait `Index` is not implemented for `[i32]` - | | - | required by a bound introduced by this call - | - = help: the trait `Index` is not implemented for `[i32]` - but trait `Index` is implemented for it - = help: for that trait implementation, expected `usize`, found `u32` - -error[E0277]: the trait bound `[i32]: Index` is not satisfied - --> $DIR/on-impl.rs:22:5 - | -LL | Index::::index(&[1, 2, 3] as &[i32], 2u32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Index` is not implemented for `[i32]` - | - = help: the trait `Index` is not implemented for `[i32]` - but trait `Index` is implemented for it - = help: for that trait implementation, expected `usize`, found `u32` - -error: aborting due to 2 previous errors - -For more information about this error, try `rustc --explain E0277`. From d1e82ba37c3dd1998808665971bbee4e1ae028b8 Mon Sep 17 00:00:00 2001 From: Thalia Archibald Date: Mon, 14 Apr 2025 21:43:53 -0700 Subject: [PATCH 13/67] Setup editor file associations for non-rs extensions .gitattributes lists *.fixed, *.pp, and *.mir as file extensions which should be treated as Rust source code. Do the same for VS Code and Zed. This only does syntax highlighting, which is appropriate, as MIR isn't really Rust code. At the same time, consistently order `rust-analyzer.linkedProjects` between editors. For some reason, Eglot didn't include library/Cargo.toml. --- src/bootstrap/src/core/build_steps/setup.rs | 3 +++ src/etc/rust_analyzer_eglot.el | 7 ++++--- src/etc/rust_analyzer_settings.json | 11 ++++++++--- src/etc/rust_analyzer_zed.json | 13 ++++++++----- 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index 80d92135dd3..83083e12ef1 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -584,6 +584,7 @@ Select which editor you would like to set up [default: None]: "; "51068d4747a13732440d1a8b8f432603badb1864fa431d83d0fd4f8fa57039e0", "d29af4d949bbe2371eac928a3c31cf9496b1701aa1c45f11cd6c759865ad5c45", "b5dd299b93dca3ceeb9b335f929293cb3d4bf4977866fbe7ceeac2a8a9f99088", + "631c837b0e98ae35fd48b0e5f743b1ca60adadf2d0a2b23566ba25df372cf1a9", ], EditorKind::Helix => &[ "2d3069b8cf1b977e5d4023965eb6199597755e6c96c185ed5f2854f98b83d233", @@ -602,10 +603,12 @@ Select which editor you would like to set up [default: None]: "; "4eecb58a2168b252077369da446c30ed0e658301efe69691979d1ef0443928f4", "c394386e6133bbf29ffd32c8af0bb3d4aac354cba9ee051f29612aa9350f8f8d", "e53e9129ca5ee5dcbd6ec8b68c2d87376474eb154992deba3c6d9ab1703e0717", + "f954316090936c7e590c253ca9d524008375882fa13c5b41d7e2547a896ff893", ], EditorKind::Zed => &[ "bbce727c269d1bd0c98afef4d612eb4ce27aea3c3a8968c5f10b31affbc40b6c", "a5380cf5dd9328731aecc5dfb240d16dac46ed272126b9728006151ef42f5909", + "2e96bf0d443852b12f016c8fc9840ab3d0a2b4fe0b0fb3a157e8d74d5e7e0e26", ], } } diff --git a/src/etc/rust_analyzer_eglot.el b/src/etc/rust_analyzer_eglot.el index 6b40371d9af..90bd38aa894 100644 --- a/src/etc/rust_analyzer_eglot.el +++ b/src/etc/rust_analyzer_eglot.el @@ -8,10 +8,11 @@ "check" "--json-output"]) :linkedProjects ["Cargo.toml" - "src/bootstrap/Cargo.toml" - "src/tools/rust-analyzer/Cargo.toml" "compiler/rustc_codegen_cranelift/Cargo.toml" - "compiler/rustc_codegen_gcc/Cargo.toml"] + "compiler/rustc_codegen_gcc/Cargo.toml" + "library/Cargo.toml" + "src/bootstrap/Cargo.toml" + "src/tools/rust-analyzer/Cargo.toml"] :rustfmt ( :overrideCommand ["build/host/rustfmt/bin/rustfmt" "--edition=2021"]) :procMacro ( :server "build/host/stage0/libexec/rust-analyzer-proc-macro-srv" diff --git a/src/etc/rust_analyzer_settings.json b/src/etc/rust_analyzer_settings.json index da7d326a512..5ce886a9b65 100644 --- a/src/etc/rust_analyzer_settings.json +++ b/src/etc/rust_analyzer_settings.json @@ -9,11 +9,11 @@ ], "rust-analyzer.linkedProjects": [ "Cargo.toml", + "compiler/rustc_codegen_cranelift/Cargo.toml", + "compiler/rustc_codegen_gcc/Cargo.toml", "library/Cargo.toml", "src/bootstrap/Cargo.toml", - "src/tools/rust-analyzer/Cargo.toml", - "compiler/rustc_codegen_cranelift/Cargo.toml", - "compiler/rustc_codegen_gcc/Cargo.toml" + "src/tools/rust-analyzer/Cargo.toml" ], "rust-analyzer.rustfmt.overrideCommand": [ "${workspaceFolder}/build/host/rustfmt/bin/rustfmt", @@ -36,5 +36,10 @@ }, "rust-analyzer.server.extraEnv": { "RUSTUP_TOOLCHAIN": "nightly" + }, + "files.associations": { + "*.fixed": "rust", + "*.pp": "rust", + "*.mir": "rust" } } diff --git a/src/etc/rust_analyzer_zed.json b/src/etc/rust_analyzer_zed.json index abc6ddbc213..3461ff887d9 100644 --- a/src/etc/rust_analyzer_zed.json +++ b/src/etc/rust_analyzer_zed.json @@ -21,15 +21,15 @@ }, "linkedProjects": [ "Cargo.toml", + "compiler/rustc_codegen_cranelift/Cargo.toml", + "compiler/rustc_codegen_gcc/Cargo.toml", "library/Cargo.toml", "src/bootstrap/Cargo.toml", - "src/tools/rust-analyzer/Cargo.toml", - "compiler/rustc_codegen_cranelift/Cargo.toml", - "compiler/rustc_codegen_gcc/Cargo.toml" + "src/tools/rust-analyzer/Cargo.toml" ], "procMacro": { - "enable": true, - "server": "${workspaceFolder}/build/host/stage0/libexec/rust-analyzer-proc-macro-srv" + "enable": true, + "server": "${workspaceFolder}/build/host/stage0/libexec/rust-analyzer-proc-macro-srv" }, "rustc": { "source": "./Cargo.toml" @@ -47,5 +47,8 @@ } } } + }, + "file_types": { + "Rust": ["fixed", "pp", "mir"] } } From 1c1febc59db038876d7fe78a1f056bf324fdff6a Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 1 Apr 2025 11:19:57 +0300 Subject: [PATCH 14/67] add new config option: `include` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 25 ++++++++++++++++++++++- src/bootstrap/src/utils/change_tracker.rs | 5 +++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 25ec64f90b5..2e881081290 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -701,6 +701,7 @@ pub(crate) struct TomlConfig { target: Option>, dist: Option, profile: Option, + include: Option>, } /// This enum is used for deserializing change IDs from TOML, allowing both numeric values and the string `"ignore"`. @@ -753,7 +754,7 @@ trait Merge { impl Merge for TomlConfig { fn merge( &mut self, - TomlConfig { build, install, llvm, gcc, rust, dist, target, profile, change_id }: Self, + TomlConfig { build, install, llvm, gcc, rust, dist, target, profile, change_id, include }: Self, replace: ReplaceOpt, ) { fn do_merge(x: &mut Option, y: Option, replace: ReplaceOpt) { @@ -766,6 +767,17 @@ impl Merge for TomlConfig { } } + for include_path in include.clone().unwrap_or_default() { + let included_toml = Config::get_toml(&include_path).unwrap_or_else(|e| { + eprintln!( + "ERROR: Failed to parse default config profile at '{}': {e}", + include_path.display() + ); + exit!(2); + }); + self.merge(included_toml, ReplaceOpt::Override); + } + self.change_id.inner.merge(change_id.inner, replace); self.profile.merge(profile, replace); @@ -1600,6 +1612,17 @@ impl Config { toml.merge(included_toml, ReplaceOpt::IgnoreDuplicate); } + for include_path in toml.include.clone().unwrap_or_default() { + let included_toml = get_toml(&include_path).unwrap_or_else(|e| { + eprintln!( + "ERROR: Failed to parse default config profile at '{}': {e}", + include_path.display() + ); + exit!(2); + }); + toml.merge(included_toml, ReplaceOpt::Override); + } + let mut override_toml = TomlConfig::default(); for option in flags.set.iter() { fn get_table(option: &str) -> Result { diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 48b6f77e8a5..3f1885a425f 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -396,4 +396,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ severity: ChangeSeverity::Info, summary: "Added a new option `build.compiletest-use-stage0-libtest` to force `compiletest` to use the stage 0 libtest.", }, + ChangeInfo { + change_id: 138934, + severity: ChangeSeverity::Info, + summary: "Added new option `include` to create config extensions.", + }, ]; From 89e3befe63f35b7556614d85ce63214f62a9a771 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 25 Mar 2025 20:17:32 +0300 Subject: [PATCH 15/67] document config extensions Signed-off-by: onur-ozkan --- .../rustc-dev-guide/src/building/suggested.md | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/doc/rustc-dev-guide/src/building/suggested.md b/src/doc/rustc-dev-guide/src/building/suggested.md index 43ff2ba726f..5d37a78af88 100644 --- a/src/doc/rustc-dev-guide/src/building/suggested.md +++ b/src/doc/rustc-dev-guide/src/building/suggested.md @@ -20,6 +20,42 @@ your `.git/hooks` folder as `pre-push` (without the `.sh` extension!). You can also install the hook as a step of running `./x setup`! +## Config extensions + +When working on different tasks, you might need to switch between different bootstrap configurations. +Sometimes you may want to keep an old configuration for future use. But saving raw config values in +random files and manually copying and pasting them can quickly become messy, especially if you have a +long history of different configurations. + +To simplify managing multiple configurations, you can create config extensions. + +For example, you can create a simple config file named `cross.toml`: + +```toml +[build] +build = "x86_64-unknown-linux-gnu" +host = ["i686-unknown-linux-gnu"] +target = ["i686-unknown-linux-gnu"] + + +[llvm] +download-ci-llvm = false + +[target.x86_64-unknown-linux-gnu] +llvm-config = "/path/to/llvm-19/bin/llvm-config" +``` + +Then, include this in your `bootstrap.toml`: + +```toml +include = ["cross.toml"] +``` + +You can also include extensions within extensions recursively. + +**Note:** In the `include` field, the overriding logic follows a right-to-left order. Also, the outer +extension/config always overrides the inner ones. + ## Configuring `rust-analyzer` for `rustc` ### Project-local rust-analyzer setup From 4e80659b32a794513646b93890c9a6eeb103de17 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 1 Apr 2025 11:44:33 +0300 Subject: [PATCH 16/67] implement cyclic inclusion handling Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 51 ++++++++++++++++++------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 2e881081290..86566d0eee1 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -6,6 +6,7 @@ use std::cell::{Cell, RefCell}; use std::collections::{BTreeSet, HashMap, HashSet}; use std::fmt::{self, Display}; +use std::hash::Hash; use std::io::IsTerminal; use std::path::{Path, PathBuf, absolute}; use std::process::Command; @@ -748,19 +749,25 @@ enum ReplaceOpt { } trait Merge { - fn merge(&mut self, other: Self, replace: ReplaceOpt); + fn merge( + &mut self, + included_extensions: &mut HashSet, + other: Self, + replace: ReplaceOpt, + ); } impl Merge for TomlConfig { fn merge( &mut self, + included_extensions: &mut HashSet, TomlConfig { build, install, llvm, gcc, rust, dist, target, profile, change_id, include }: Self, replace: ReplaceOpt, ) { fn do_merge(x: &mut Option, y: Option, replace: ReplaceOpt) { if let Some(new) = y { if let Some(original) = x { - original.merge(new, replace); + original.merge(&mut Default::default(), new, replace); } else { *x = Some(new); } @@ -775,11 +782,20 @@ impl Merge for TomlConfig { ); exit!(2); }); - self.merge(included_toml, ReplaceOpt::Override); + + assert!( + included_extensions.insert(include_path.clone()), + "Cyclic inclusion detected: '{}' is being included again before its previous inclusion was fully processed.", + include_path.display() + ); + + self.merge(included_extensions, included_toml, ReplaceOpt::Override); + + included_extensions.remove(&include_path); } - self.change_id.inner.merge(change_id.inner, replace); - self.profile.merge(profile, replace); + self.change_id.inner.merge(&mut Default::default(), change_id.inner, replace); + self.profile.merge(&mut Default::default(), profile, replace); do_merge(&mut self.build, build, replace); do_merge(&mut self.install, install, replace); @@ -794,7 +810,7 @@ impl Merge for TomlConfig { (Some(original_target), Some(new_target)) => { for (triple, new) in new_target { if let Some(original) = original_target.get_mut(&triple) { - original.merge(new, replace); + original.merge(&mut Default::default(), new, replace); } else { original_target.insert(triple, new); } @@ -815,7 +831,7 @@ macro_rules! define_config { } impl Merge for $name { - fn merge(&mut self, other: Self, replace: ReplaceOpt) { + fn merge(&mut self, _included_extensions: &mut HashSet, other: Self, replace: ReplaceOpt) { $( match replace { ReplaceOpt::IgnoreDuplicate => { @@ -915,7 +931,12 @@ macro_rules! define_config { } impl Merge for Option { - fn merge(&mut self, other: Self, replace: ReplaceOpt) { + fn merge( + &mut self, + _included_extensions: &mut HashSet, + other: Self, + replace: ReplaceOpt, + ) { match replace { ReplaceOpt::IgnoreDuplicate => { if self.is_none() { @@ -1609,7 +1630,7 @@ impl Config { ); exit!(2); }); - toml.merge(included_toml, ReplaceOpt::IgnoreDuplicate); + toml.merge(&mut Default::default(), included_toml, ReplaceOpt::IgnoreDuplicate); } for include_path in toml.include.clone().unwrap_or_default() { @@ -1620,7 +1641,7 @@ impl Config { ); exit!(2); }); - toml.merge(included_toml, ReplaceOpt::Override); + toml.merge(&mut Default::default(), included_toml, ReplaceOpt::Override); } let mut override_toml = TomlConfig::default(); @@ -1631,7 +1652,7 @@ impl Config { let mut err = match get_table(option) { Ok(v) => { - override_toml.merge(v, ReplaceOpt::ErrorOnDuplicate); + override_toml.merge(&mut Default::default(), v, ReplaceOpt::ErrorOnDuplicate); continue; } Err(e) => e, @@ -1642,7 +1663,11 @@ impl Config { if !value.contains('"') { match get_table(&format!(r#"{key}="{value}""#)) { Ok(v) => { - override_toml.merge(v, ReplaceOpt::ErrorOnDuplicate); + override_toml.merge( + &mut Default::default(), + v, + ReplaceOpt::ErrorOnDuplicate, + ); continue; } Err(e) => err = e, @@ -1652,7 +1677,7 @@ impl Config { eprintln!("failed to parse override `{option}`: `{err}"); exit!(2) } - toml.merge(override_toml, ReplaceOpt::Override); + toml.merge(&mut Default::default(), override_toml, ReplaceOpt::Override); config.change_id = toml.change_id.inner; From 78cb4538ee203b08441382b5cd64b1989ed5b3b9 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 1 Apr 2025 11:53:32 +0300 Subject: [PATCH 17/67] document `include` in `bootstrap.example.toml` Signed-off-by: onur-ozkan --- bootstrap.example.toml | 5 +++++ src/doc/rustc-dev-guide/src/building/suggested.md | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/bootstrap.example.toml b/bootstrap.example.toml index 0927f648635..0a314f65ec2 100644 --- a/bootstrap.example.toml +++ b/bootstrap.example.toml @@ -19,6 +19,11 @@ # Note that this has no default value (x.py uses the defaults in `bootstrap.example.toml`). #profile = +# Inherits configuration values from different configuration files (a.k.a. config extensions). +# Supports absolute paths, and uses the current directory (where the bootstrap was invoked) +# as the base if the given path is not absolute. +#include = [] + # Keeps track of major changes made to this configuration. # # This value also represents ID of the PR that caused major changes. Meaning, diff --git a/src/doc/rustc-dev-guide/src/building/suggested.md b/src/doc/rustc-dev-guide/src/building/suggested.md index 5d37a78af88..b2e258be079 100644 --- a/src/doc/rustc-dev-guide/src/building/suggested.md +++ b/src/doc/rustc-dev-guide/src/building/suggested.md @@ -53,8 +53,9 @@ include = ["cross.toml"] You can also include extensions within extensions recursively. -**Note:** In the `include` field, the overriding logic follows a right-to-left order. Also, the outer -extension/config always overrides the inner ones. +**Note:** In the `include` field, the overriding logic follows a right-to-left order. For example, +in `include = ["a.toml", "b.toml"]`, extension `b.toml` overrides `a.toml`. Also, parent extensions +always overrides the inner ones. ## Configuring `rust-analyzer` for `rustc` From 3f70f197f2d131a1f57912f202072b2447d3e326 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 1 Apr 2025 21:56:58 +0000 Subject: [PATCH 18/67] apply nit notes Signed-off-by: onur-ozkan --- bootstrap.example.toml | 3 + src/bootstrap/src/core/config/config.rs | 95 ++++++++++++++++++------- 2 files changed, 72 insertions(+), 26 deletions(-) diff --git a/bootstrap.example.toml b/bootstrap.example.toml index 0a314f65ec2..72c4492d465 100644 --- a/bootstrap.example.toml +++ b/bootstrap.example.toml @@ -22,6 +22,9 @@ # Inherits configuration values from different configuration files (a.k.a. config extensions). # Supports absolute paths, and uses the current directory (where the bootstrap was invoked) # as the base if the given path is not absolute. +# +# The overriding logic follows a right-to-left order. For example, in `include = ["a.toml", "b.toml"]`, +# extension `b.toml` overrides `a.toml`. Also, parent extensions always overrides the inner ones. #include = [] # Keeps track of major changes made to this configuration. diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 86566d0eee1..e70ac74c7da 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -751,6 +751,7 @@ enum ReplaceOpt { trait Merge { fn merge( &mut self, + parent_config_path: Option, included_extensions: &mut HashSet, other: Self, replace: ReplaceOpt, @@ -760,6 +761,7 @@ trait Merge { impl Merge for TomlConfig { fn merge( &mut self, + parent_config_path: Option, included_extensions: &mut HashSet, TomlConfig { build, install, llvm, gcc, rust, dist, target, profile, change_id, include }: Self, replace: ReplaceOpt, @@ -767,19 +769,27 @@ impl Merge for TomlConfig { fn do_merge(x: &mut Option, y: Option, replace: ReplaceOpt) { if let Some(new) = y { if let Some(original) = x { - original.merge(&mut Default::default(), new, replace); + original.merge(None, &mut Default::default(), new, replace); } else { *x = Some(new); } } } - for include_path in include.clone().unwrap_or_default() { + let parent_dir = parent_config_path + .as_ref() + .and_then(|p| p.parent().map(ToOwned::to_owned)) + .unwrap_or_default(); + + for include_path in include.clone().unwrap_or_default().iter().rev() { + let include_path = parent_dir.join(include_path); + let include_path = include_path.canonicalize().unwrap_or_else(|e| { + eprintln!("ERROR: Failed to canonicalize '{}' path: {e}", include_path.display()); + exit!(2); + }); + let included_toml = Config::get_toml(&include_path).unwrap_or_else(|e| { - eprintln!( - "ERROR: Failed to parse default config profile at '{}': {e}", - include_path.display() - ); + eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); exit!(2); }); @@ -789,13 +799,20 @@ impl Merge for TomlConfig { include_path.display() ); - self.merge(included_extensions, included_toml, ReplaceOpt::Override); + self.merge( + Some(include_path.clone()), + included_extensions, + included_toml, + // Ensures that parent configuration always takes precedence + // over child configurations. + ReplaceOpt::IgnoreDuplicate, + ); included_extensions.remove(&include_path); } - self.change_id.inner.merge(&mut Default::default(), change_id.inner, replace); - self.profile.merge(&mut Default::default(), profile, replace); + self.change_id.inner.merge(None, &mut Default::default(), change_id.inner, replace); + self.profile.merge(None, &mut Default::default(), profile, replace); do_merge(&mut self.build, build, replace); do_merge(&mut self.install, install, replace); @@ -810,7 +827,7 @@ impl Merge for TomlConfig { (Some(original_target), Some(new_target)) => { for (triple, new) in new_target { if let Some(original) = original_target.get_mut(&triple) { - original.merge(&mut Default::default(), new, replace); + original.merge(None, &mut Default::default(), new, replace); } else { original_target.insert(triple, new); } @@ -831,7 +848,13 @@ macro_rules! define_config { } impl Merge for $name { - fn merge(&mut self, _included_extensions: &mut HashSet, other: Self, replace: ReplaceOpt) { + fn merge( + &mut self, + _parent_config_path: Option, + _included_extensions: &mut HashSet, + other: Self, + replace: ReplaceOpt + ) { $( match replace { ReplaceOpt::IgnoreDuplicate => { @@ -933,6 +956,7 @@ macro_rules! define_config { impl Merge for Option { fn merge( &mut self, + _parent_config_path: Option, _included_extensions: &mut HashSet, other: Self, replace: ReplaceOpt, @@ -1581,7 +1605,8 @@ impl Config { // but not if `bootstrap.toml` hasn't been created. let mut toml = if !using_default_path || toml_path.exists() { config.config = Some(if cfg!(not(test)) { - toml_path.canonicalize().unwrap() + toml_path = toml_path.canonicalize().unwrap(); + toml_path.clone() } else { toml_path.clone() }); @@ -1609,6 +1634,24 @@ impl Config { toml.profile = Some("dist".into()); } + // Reverse the list to ensure the last added config extension remains the most dominant. + // For example, given ["a.toml", "b.toml"], "b.toml" should take precedence over "a.toml". + // + // This must be handled before applying the `profile` since `include`s should always take + // precedence over `profile`s. + for include_path in toml.include.clone().unwrap_or_default().iter().rev() { + let included_toml = get_toml(include_path).unwrap_or_else(|e| { + eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); + exit!(2); + }); + toml.merge( + Some(toml_path.join(include_path)), + &mut Default::default(), + included_toml, + ReplaceOpt::IgnoreDuplicate, + ); + } + if let Some(include) = &toml.profile { // Allows creating alias for profile names, allowing // profiles to be renamed while maintaining back compatibility @@ -1630,18 +1673,12 @@ impl Config { ); exit!(2); }); - toml.merge(&mut Default::default(), included_toml, ReplaceOpt::IgnoreDuplicate); - } - - for include_path in toml.include.clone().unwrap_or_default() { - let included_toml = get_toml(&include_path).unwrap_or_else(|e| { - eprintln!( - "ERROR: Failed to parse default config profile at '{}': {e}", - include_path.display() - ); - exit!(2); - }); - toml.merge(&mut Default::default(), included_toml, ReplaceOpt::Override); + toml.merge( + Some(include_path), + &mut Default::default(), + included_toml, + ReplaceOpt::IgnoreDuplicate, + ); } let mut override_toml = TomlConfig::default(); @@ -1652,7 +1689,12 @@ impl Config { let mut err = match get_table(option) { Ok(v) => { - override_toml.merge(&mut Default::default(), v, ReplaceOpt::ErrorOnDuplicate); + override_toml.merge( + None, + &mut Default::default(), + v, + ReplaceOpt::ErrorOnDuplicate, + ); continue; } Err(e) => e, @@ -1664,6 +1706,7 @@ impl Config { match get_table(&format!(r#"{key}="{value}""#)) { Ok(v) => { override_toml.merge( + None, &mut Default::default(), v, ReplaceOpt::ErrorOnDuplicate, @@ -1677,7 +1720,7 @@ impl Config { eprintln!("failed to parse override `{option}`: `{err}"); exit!(2) } - toml.merge(&mut Default::default(), override_toml, ReplaceOpt::Override); + toml.merge(None, &mut Default::default(), override_toml, ReplaceOpt::Override); config.change_id = toml.change_id.inner; From 8e6f50bb4d30801d04619a4cd6406b320eb8919f Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 15 Apr 2025 10:44:19 +0300 Subject: [PATCH 19/67] fix path and the ordering logic Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 54 +++++++++++++------------ 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index e70ac74c7da..19ed442c500 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -776,6 +776,30 @@ impl Merge for TomlConfig { } } + self.change_id.inner.merge(None, &mut Default::default(), change_id.inner, replace); + self.profile.merge(None, &mut Default::default(), profile, replace); + + do_merge(&mut self.build, build, replace); + do_merge(&mut self.install, install, replace); + do_merge(&mut self.llvm, llvm, replace); + do_merge(&mut self.gcc, gcc, replace); + do_merge(&mut self.rust, rust, replace); + do_merge(&mut self.dist, dist, replace); + + match (self.target.as_mut(), target) { + (_, None) => {} + (None, Some(target)) => self.target = Some(target), + (Some(original_target), Some(new_target)) => { + for (triple, new) in new_target { + if let Some(original) = original_target.get_mut(&triple) { + original.merge(None, &mut Default::default(), new, replace); + } else { + original_target.insert(triple, new); + } + } + } + } + let parent_dir = parent_config_path .as_ref() .and_then(|p| p.parent().map(ToOwned::to_owned)) @@ -810,30 +834,6 @@ impl Merge for TomlConfig { included_extensions.remove(&include_path); } - - self.change_id.inner.merge(None, &mut Default::default(), change_id.inner, replace); - self.profile.merge(None, &mut Default::default(), profile, replace); - - do_merge(&mut self.build, build, replace); - do_merge(&mut self.install, install, replace); - do_merge(&mut self.llvm, llvm, replace); - do_merge(&mut self.gcc, gcc, replace); - do_merge(&mut self.rust, rust, replace); - do_merge(&mut self.dist, dist, replace); - - match (self.target.as_mut(), target) { - (_, None) => {} - (None, Some(target)) => self.target = Some(target), - (Some(original_target), Some(new_target)) => { - for (triple, new) in new_target { - if let Some(original) = original_target.get_mut(&triple) { - original.merge(None, &mut Default::default(), new, replace); - } else { - original_target.insert(triple, new); - } - } - } - } } } @@ -1640,12 +1640,14 @@ impl Config { // This must be handled before applying the `profile` since `include`s should always take // precedence over `profile`s. for include_path in toml.include.clone().unwrap_or_default().iter().rev() { - let included_toml = get_toml(include_path).unwrap_or_else(|e| { + let include_path = toml_path.parent().unwrap().join(include_path); + + let included_toml = get_toml(&include_path).unwrap_or_else(|e| { eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); exit!(2); }); toml.merge( - Some(toml_path.join(include_path)), + Some(include_path), &mut Default::default(), included_toml, ReplaceOpt::IgnoreDuplicate, From 7dfb457745d7f873eb5db1c3fb39d58fa768740d Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 15 Apr 2025 11:53:21 +0300 Subject: [PATCH 20/67] add FIXME note in `TomlConfig::merge` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 19ed442c500..b7afd81dfdb 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -812,6 +812,8 @@ impl Merge for TomlConfig { exit!(2); }); + // FIXME: Similar to `Config::parse_inner`, allow passing a custom `get_toml` from the caller to + // improve testability since `Config::get_toml` does nothing when `cfg(test)` is enabled. let included_toml = Config::get_toml(&include_path).unwrap_or_else(|e| { eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); exit!(2); From 6d52b51d3e6321b7a1d24e5f995aa709057153e3 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 15 Apr 2025 19:10:31 +0300 Subject: [PATCH 21/67] add comment in `TomlConfig::merge` about the merge order Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index b7afd81dfdb..e15aab4ad50 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -805,6 +805,8 @@ impl Merge for TomlConfig { .and_then(|p| p.parent().map(ToOwned::to_owned)) .unwrap_or_default(); + // `include` handled later since we ignore duplicates using `ReplaceOpt::IgnoreDuplicate` to + // keep the upper-level configuration to take precedence. for include_path in include.clone().unwrap_or_default().iter().rev() { let include_path = parent_dir.join(include_path); let include_path = include_path.canonicalize().unwrap_or_else(|e| { From 2f0ba6791944fbbc9236ea4138389039ebbffd4a Mon Sep 17 00:00:00 2001 From: Lyndon Brown Date: Wed, 16 Apr 2025 14:42:11 +0100 Subject: [PATCH 22/67] fix incorrect type in cstr `to_string_lossy()` docs Restoring what it said prior to commit 67065fe in which it was changed incorrectly with no supporting explanation. Closes #139835. --- library/alloc/src/ffi/c_str.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/ffi/c_str.rs b/library/alloc/src/ffi/c_str.rs index f6743c65710..ef8548d2429 100644 --- a/library/alloc/src/ffi/c_str.rs +++ b/library/alloc/src/ffi/c_str.rs @@ -1116,7 +1116,7 @@ impl CStr { /// with the corresponding &[str] slice. Otherwise, it will /// replace any invalid UTF-8 sequences with /// [`U+FFFD REPLACEMENT CHARACTER`][U+FFFD] and return a - /// [Cow]::[Owned]\(&[str]) with the result. + /// [Cow]::[Owned]\([String]) with the result. /// /// [str]: prim@str "str" /// [Borrowed]: Cow::Borrowed From 2024e26881ea8bdfb41f53e257464be4332abc79 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 15 Apr 2025 01:01:51 +0000 Subject: [PATCH 23/67] Don't canonicalize crate paths --- compiler/rustc_metadata/src/locator.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs index 112954eca0d..f0a898d678c 100644 --- a/compiler/rustc_metadata/src/locator.rs +++ b/compiler/rustc_metadata/src/locator.rs @@ -427,12 +427,21 @@ impl<'a> CrateLocator<'a> { let (rlibs, rmetas, dylibs) = candidates.entry(hash.to_string()).or_default(); - let path = - try_canonicalize(&spf.path).unwrap_or_else(|_| spf.path.to_path_buf()); - if seen_paths.contains(&path) { - continue; - }; - seen_paths.insert(path.clone()); + { + // As a perforamnce optimisation we canonicalize the path and skip + // ones we've already seeen. This allows us to ignore crates + // we know are exactual equal to ones we've already found. + // Going to the same crate through different symlinks does not change the result. + let path = try_canonicalize(&spf.path) + .unwrap_or_else(|_| spf.path.to_path_buf()); + if seen_paths.contains(&path) { + continue; + }; + seen_paths.insert(path); + } + // Use the original path (potentially with unresolved symlinks), + // filesystem code should not care, but this is nicer for diagnostics. + let path = spf.path.to_path_buf(); match kind { CrateFlavor::Rlib => rlibs.insert(path, search_path.kind), CrateFlavor::Rmeta => rmetas.insert(path, search_path.kind), From 52f35d013116559035621eb83d0e9680ecf74a49 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 15 Apr 2025 12:55:33 +0000 Subject: [PATCH 24/67] Test for relative paths in crate path diagnostics --- .../crateresolve1-1.rs | 6 ++++ .../crateresolve1-2.rs | 6 ++++ .../multiple-candidates.rs | 3 ++ .../multiple-candidates.stderr | 12 +++++++ .../rmake.rs | 34 +++++++++++++++++++ 5 files changed, 61 insertions(+) create mode 100644 tests/run-make/crate-loading-multiple-candidates/crateresolve1-1.rs create mode 100644 tests/run-make/crate-loading-multiple-candidates/crateresolve1-2.rs create mode 100644 tests/run-make/crate-loading-multiple-candidates/multiple-candidates.rs create mode 100644 tests/run-make/crate-loading-multiple-candidates/multiple-candidates.stderr create mode 100644 tests/run-make/crate-loading-multiple-candidates/rmake.rs diff --git a/tests/run-make/crate-loading-multiple-candidates/crateresolve1-1.rs b/tests/run-make/crate-loading-multiple-candidates/crateresolve1-1.rs new file mode 100644 index 00000000000..fe00f041a86 --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/crateresolve1-1.rs @@ -0,0 +1,6 @@ +#![crate_name = "crateresolve1"] +#![crate_type = "lib"] + +pub fn f() -> isize { + 10 +} diff --git a/tests/run-make/crate-loading-multiple-candidates/crateresolve1-2.rs b/tests/run-make/crate-loading-multiple-candidates/crateresolve1-2.rs new file mode 100644 index 00000000000..0fb8591b3a5 --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/crateresolve1-2.rs @@ -0,0 +1,6 @@ +#![crate_name = "crateresolve1"] +#![crate_type = "lib"] + +pub fn f() -> isize { + 20 +} diff --git a/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.rs b/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.rs new file mode 100644 index 00000000000..27cd7ca5c20 --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.rs @@ -0,0 +1,3 @@ +extern crate crateresolve1; + +fn main() {} diff --git a/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.stderr b/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.stderr new file mode 100644 index 00000000000..de7fc3b0feb --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.stderr @@ -0,0 +1,12 @@ +error[E0464]: multiple candidates for `rlib` dependency `crateresolve1` found + --> multiple-candidates.rs:1:1 + | +LL | extern crate crateresolve1; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: candidate #1: ./mylibs/libcrateresolve1-1.rlib + = note: candidate #2: ./mylibs/libcrateresolve1-2.rlib + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0464`. diff --git a/tests/run-make/crate-loading-multiple-candidates/rmake.rs b/tests/run-make/crate-loading-multiple-candidates/rmake.rs new file mode 100644 index 00000000000..ce090850500 --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/rmake.rs @@ -0,0 +1,34 @@ +//@ needs-symlink +//@ ignore-cross-compile + +// Tests that the multiple candidate dependencies diagnostic prints relative +// paths if a relative library path was passed in. + +use run_make_support::{bare_rustc, diff, rfs, rustc}; + +fn main() { + // Check that relative paths are preserved in the diagnostic + rfs::create_dir("mylibs"); + rustc().input("crateresolve1-1.rs").out_dir("mylibs").extra_filename("-1").run(); + rustc().input("crateresolve1-2.rs").out_dir("mylibs").extra_filename("-2").run(); + check("./mylibs"); + + // Check that symlinks aren't followed when printing the diagnostic + rfs::rename("mylibs", "original"); + rfs::symlink_dir("original", "mylibs"); + check("./mylibs"); +} + +fn check(library_path: &str) { + let out = rustc() + .input("multiple-candidates.rs") + .library_search_path(library_path) + .ui_testing() + .run_fail() + .stderr_utf8(); + diff() + .expected_file("multiple-candidates.stderr") + .normalize(r"\\", "/") + .actual_text("(rustc)", &out) + .run(); +} From 827047895a4c036f652b7105443f6305164f0e25 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 16 Apr 2025 20:53:51 +0300 Subject: [PATCH 25/67] resolve config include FIXME Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index e15aab4ad50..0c221abc969 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -814,9 +814,7 @@ impl Merge for TomlConfig { exit!(2); }); - // FIXME: Similar to `Config::parse_inner`, allow passing a custom `get_toml` from the caller to - // improve testability since `Config::get_toml` does nothing when `cfg(test)` is enabled. - let included_toml = Config::get_toml(&include_path).unwrap_or_else(|e| { + let included_toml = Config::get_toml_inner(&include_path).unwrap_or_else(|e| { eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); exit!(2); }); @@ -1424,13 +1422,15 @@ impl Config { Self::get_toml(&builder_config_path) } - #[cfg(test)] - pub(crate) fn get_toml(_: &Path) -> Result { - Ok(TomlConfig::default()) + pub(crate) fn get_toml(file: &Path) -> Result { + #[cfg(test)] + return Ok(TomlConfig::default()); + + #[cfg(not(test))] + Self::get_toml_inner(file) } - #[cfg(not(test))] - pub(crate) fn get_toml(file: &Path) -> Result { + fn get_toml_inner(file: &Path) -> Result { let contents = t!(fs::read_to_string(file), format!("config file {} not found", file.display())); // Deserialize to Value and then TomlConfig to prevent the Deserialize impl of From 5a38550a39a99b1cd98f9be769e4f1face3d1256 Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Wed, 2 Apr 2025 20:33:37 -0400 Subject: [PATCH 26/67] Deduplicate nix code And clean it up a little. --- src/tools/nix-dev-shell/flake.nix | 46 +++++++--------- src/tools/nix-dev-shell/shell.nix | 38 +++++++------ src/tools/nix-dev-shell/x/default.nix | 77 ++++++++++++++++++++++++--- 3 files changed, 111 insertions(+), 50 deletions(-) diff --git a/src/tools/nix-dev-shell/flake.nix b/src/tools/nix-dev-shell/flake.nix index 1b838bd2f7b..b8287de5fcf 100644 --- a/src/tools/nix-dev-shell/flake.nix +++ b/src/tools/nix-dev-shell/flake.nix @@ -1,32 +1,24 @@ { description = "rustc dev shell"; - inputs = { - nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; - flake-utils.url = "github:numtide/flake-utils"; - }; + inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; - outputs = { self, nixpkgs, flake-utils, ... }: - flake-utils.lib.eachDefaultSystem (system: - let - pkgs = import nixpkgs { inherit system; }; - x = import ./x { inherit pkgs; }; - in - { - devShells.default = with pkgs; mkShell { - name = "rustc-dev-shell"; - nativeBuildInputs = with pkgs; [ - binutils cmake ninja pkg-config python3 git curl cacert patchelf nix - ]; - buildInputs = with pkgs; [ - openssl glibc.out glibc.static x - ]; - # Avoid creating text files for ICEs. - RUSTC_ICE = "0"; - # Provide `libstdc++.so.6` for the self-contained lld. - # Provide `libz.so.1`. - LD_LIBRARY_PATH = "${with pkgs; lib.makeLibraryPath [stdenv.cc.cc.lib zlib]}"; - }; - } - ); + outputs = + { + self, + nixpkgs, + }: + let + inherit (nixpkgs) lib; + forEachSystem = lib.genAttrs lib.systems.flakeExposed; + in + { + devShells = forEachSystem (system: { + default = nixpkgs.legacyPackages.${system}.callPackage ./shell.nix { }; + }); + + packages = forEachSystem (system: { + default = nixpkgs.legacyPackages.${system}.callPackage ./x { }; + }); + }; } diff --git a/src/tools/nix-dev-shell/shell.nix b/src/tools/nix-dev-shell/shell.nix index a3f5969bd81..0adbacf7e8d 100644 --- a/src/tools/nix-dev-shell/shell.nix +++ b/src/tools/nix-dev-shell/shell.nix @@ -1,18 +1,26 @@ -{ pkgs ? import {} }: -let - x = import ./x { inherit pkgs; }; +{ + pkgs ? import { }, +}: +let + inherit (pkgs.lib) lists attrsets; + + x = pkgs.callPackage ./x { }; + inherit (x.passthru) cacert env; in pkgs.mkShell { - name = "rustc"; - nativeBuildInputs = with pkgs; [ - binutils cmake ninja pkg-config python3 git curl cacert patchelf nix - ]; - buildInputs = with pkgs; [ - openssl glibc.out glibc.static x - ]; - # Avoid creating text files for ICEs. - RUSTC_ICE = "0"; - # Provide `libstdc++.so.6` for the self-contained lld. - # Provide `libz.so.1` - LD_LIBRARY_PATH = "${with pkgs; lib.makeLibraryPath [stdenv.cc.cc.lib zlib]}"; + name = "rustc-shell"; + + inputsFrom = [ x ]; + packages = [ + pkgs.git + pkgs.nix + x + # Get the runtime deps of the x wrapper + ] ++ lists.flatten (attrsets.attrValues env); + + env = { + # Avoid creating text files for ICEs. + RUSTC_ICE = 0; + SSL_CERT_FILE = cacert; + }; } diff --git a/src/tools/nix-dev-shell/x/default.nix b/src/tools/nix-dev-shell/x/default.nix index e6dfbad6f19..422c1c4a2ae 100644 --- a/src/tools/nix-dev-shell/x/default.nix +++ b/src/tools/nix-dev-shell/x/default.nix @@ -1,22 +1,83 @@ { - pkgs ? import { }, + pkgs, + lib, + stdenv, + rustc, + python3, + makeBinaryWrapper, + # Bootstrap + curl, + pkg-config, + libiconv, + openssl, + patchelf, + cacert, + zlib, + # LLVM Deps + ninja, + cmake, + glibc, }: -pkgs.stdenv.mkDerivation { - name = "x"; +stdenv.mkDerivation (self: { + strictDeps = true; + name = "x-none"; + + outputs = [ + "out" + "unwrapped" + ]; src = ./x.rs; dontUnpack = true; - nativeBuildInputs = with pkgs; [ rustc ]; + nativeBuildInputs = [ + rustc + makeBinaryWrapper + ]; + env.PYTHON = python3.interpreter; buildPhase = '' - PYTHON=${pkgs.lib.getExe pkgs.python3} rustc -Copt-level=3 --crate-name x $src --out-dir $out/bin + rustc -Copt-level=3 --crate-name x $src --out-dir $unwrapped/bin ''; - meta = with pkgs.lib; { + installPhase = + let + inherit (self.passthru) cacert env; + in + '' + makeWrapper $unwrapped/bin/x $out/bin/x \ + --set-default SSL_CERT_FILE ${cacert} \ + --prefix CPATH ";" "${lib.makeSearchPath "include" env.cpath}" \ + --prefix PATH : ${lib.makeBinPath env.path} \ + --prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath env.ldLib} + ''; + + # For accessing them in the devshell + passthru = { + env = { + cpath = [ libiconv ]; + path = [ + python3 + patchelf + curl + pkg-config + cmake + ninja + stdenv.cc + ]; + ldLib = [ + openssl + zlib + stdenv.cc.cc.lib + ]; + }; + cacert = "${cacert}/etc/ssl/certs/ca-bundle.crt"; + }; + + meta = { description = "Helper for rust-lang/rust x.py"; homepage = "https://github.com/rust-lang/rust/blob/master/src/tools/x"; - license = licenses.mit; + license = lib.licenses.mit; mainProgram = "x"; }; -} +}) From d14df2652df492fe0f19c5e0ae5041b39b5a4d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 16 Apr 2025 13:24:01 +0200 Subject: [PATCH 27/67] Make `parent` in `download_auto_job_metrics` optional --- src/ci/citool/src/main.rs | 2 +- src/ci/citool/src/metrics.rs | 23 ++++++++++++----------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/ci/citool/src/main.rs b/src/ci/citool/src/main.rs index a1956da352f..0fee862f572 100644 --- a/src/ci/citool/src/main.rs +++ b/src/ci/citool/src/main.rs @@ -180,7 +180,7 @@ fn postprocess_metrics( } fn post_merge_report(db: JobDatabase, current: String, parent: String) -> anyhow::Result<()> { - let metrics = download_auto_job_metrics(&db, &parent, ¤t)?; + let metrics = download_auto_job_metrics(&db, Some(&parent), ¤t)?; println!("\nComparing {parent} (parent) -> {current} (this PR)\n"); diff --git a/src/ci/citool/src/metrics.rs b/src/ci/citool/src/metrics.rs index a816fb3c4f1..3d8b1ad84cf 100644 --- a/src/ci/citool/src/metrics.rs +++ b/src/ci/citool/src/metrics.rs @@ -46,24 +46,25 @@ pub struct JobMetrics { /// `parent` and `current` should be commit SHAs. pub fn download_auto_job_metrics( job_db: &JobDatabase, - parent: &str, + parent: Option<&str>, current: &str, ) -> anyhow::Result> { let mut jobs = HashMap::default(); for job in &job_db.auto_jobs { eprintln!("Downloading metrics of job {}", job.name); - let metrics_parent = match download_job_metrics(&job.name, parent) { - Ok(metrics) => Some(metrics), - Err(error) => { - eprintln!( - r#"Did not find metrics for job `{}` at `{parent}`: {error:?}. + let metrics_parent = + parent.and_then(|parent| match download_job_metrics(&job.name, parent) { + Ok(metrics) => Some(metrics), + Err(error) => { + eprintln!( + r#"Did not find metrics for job `{}` at `{parent}`: {error:?}. Maybe it was newly added?"#, - job.name - ); - None - } - }; + job.name + ); + None + } + }); let metrics_current = download_job_metrics(&job.name, current)?; jobs.insert( job.name.clone(), From 111c15c48e618b30a0cf11d91b135d87d73053a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 16 Apr 2025 17:20:53 +0200 Subject: [PATCH 28/67] Extract function for normalizing path delimiters to `utils` --- src/ci/citool/src/analysis.rs | 13 ++++++------- src/ci/citool/src/utils.rs | 6 ++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/ci/citool/src/analysis.rs b/src/ci/citool/src/analysis.rs index 9fc7c309bfb..62974be2dbe 100644 --- a/src/ci/citool/src/analysis.rs +++ b/src/ci/citool/src/analysis.rs @@ -8,9 +8,9 @@ use build_helper::metrics::{ }; use crate::github::JobInfoResolver; -use crate::metrics; use crate::metrics::{JobMetrics, JobName, get_test_suites}; use crate::utils::{output_details, pluralize}; +use crate::{metrics, utils}; /// Outputs durations of individual bootstrap steps from the gathered bootstrap invocations, /// and also a table with summarized information about executed tests. @@ -394,18 +394,17 @@ fn aggregate_tests(metrics: &JsonRoot) -> TestSuiteData { // Poor man's detection of doctests based on the "(line XYZ)" suffix let is_doctest = matches!(suite.metadata, TestSuiteMetadata::CargoPackage { .. }) && test.name.contains("(line"); - let test_entry = Test { name: generate_test_name(&test.name), stage, is_doctest }; + let test_entry = Test { + name: utils::normalize_path_delimiters(&test.name).to_string(), + stage, + is_doctest, + }; tests.insert(test_entry, test.outcome.clone()); } } TestSuiteData { tests } } -/// Normalizes Windows-style path delimiters to Unix-style paths. -fn generate_test_name(name: &str) -> String { - name.replace('\\', "/") -} - /// Prints test changes in Markdown format to stdout. fn report_test_diffs( diff: AggregatedTestDiffs, diff --git a/src/ci/citool/src/utils.rs b/src/ci/citool/src/utils.rs index a4c6ff85ef7..0367d349a1e 100644 --- a/src/ci/citool/src/utils.rs +++ b/src/ci/citool/src/utils.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::path::Path; use anyhow::Context; @@ -28,3 +29,8 @@ where func(); println!("\n"); } + +/// Normalizes Windows-style path delimiters to Unix-style paths. +pub fn normalize_path_delimiters(name: &str) -> Cow { + if name.contains("\\") { name.replace('\\', "/").into() } else { name.into() } +} From c8a882b7b58a7b8f6b276ab64117b55bbe2626e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 09:41:12 +0200 Subject: [PATCH 29/67] Add command to `citool` for generating a test dashboard --- src/ci/citool/Cargo.lock | 67 ++++++ src/ci/citool/Cargo.toml | 1 + src/ci/citool/src/main.rs | 16 +- src/ci/citool/src/test_dashboard/mod.rs | 239 +++++++++++++++++++++ src/ci/citool/templates/layout.askama | 71 ++++++ src/ci/citool/templates/test_group.askama | 22 ++ src/ci/citool/templates/test_suites.askama | 18 ++ 7 files changed, 433 insertions(+), 1 deletion(-) create mode 100644 src/ci/citool/src/test_dashboard/mod.rs create mode 100644 src/ci/citool/templates/layout.askama create mode 100644 src/ci/citool/templates/test_group.askama create mode 100644 src/ci/citool/templates/test_suites.askama diff --git a/src/ci/citool/Cargo.lock b/src/ci/citool/Cargo.lock index 2fe219f368b..43321d12caf 100644 --- a/src/ci/citool/Cargo.lock +++ b/src/ci/citool/Cargo.lock @@ -64,12 +64,63 @@ version = "1.0.95" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34ac096ce696dc2fcabef30516bb13c0a68a11d30131d3df6f04711467681b04" +[[package]] +name = "askama" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d4744ed2eef2645831b441d8f5459689ade2ab27c854488fbab1fbe94fce1a7" +dependencies = [ + "askama_derive", + "itoa", + "percent-encoding", + "serde", + "serde_json", +] + +[[package]] +name = "askama_derive" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d661e0f57be36a5c14c48f78d09011e67e0cb618f269cca9f2fd8d15b68c46ac" +dependencies = [ + "askama_parser", + "basic-toml", + "memchr", + "proc-macro2", + "quote", + "rustc-hash", + "serde", + "serde_derive", + "syn", +] + +[[package]] +name = "askama_parser" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf315ce6524c857bb129ff794935cf6d42c82a6cff60526fe2a63593de4d0d4f" +dependencies = [ + "memchr", + "serde", + "serde_derive", + "winnow", +] + [[package]] name = "base64" version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "basic-toml" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba62675e8242a4c4e806d12f11d136e626e6c8361d6b829310732241652a178a" +dependencies = [ + "serde", +] + [[package]] name = "build_helper" version = "0.1.0" @@ -104,6 +155,7 @@ name = "citool" version = "0.1.0" dependencies = [ "anyhow", + "askama", "build_helper", "clap", "csv", @@ -646,6 +698,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rustc-hash" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "357703d41365b4b27c590e3ed91eabb1b663f07c4c084095e60cbed4362dff0d" + [[package]] name = "rustls" version = "0.23.23" @@ -1026,6 +1084,15 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "winnow" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63d3fcd9bba44b03821e7d699eeee959f3126dcc4aa8e4ae18ec617c2a5cea10" +dependencies = [ + "memchr", +] + [[package]] name = "write16" version = "1.0.0" diff --git a/src/ci/citool/Cargo.toml b/src/ci/citool/Cargo.toml index f18436a1263..0e2aba3b9e3 100644 --- a/src/ci/citool/Cargo.toml +++ b/src/ci/citool/Cargo.toml @@ -5,6 +5,7 @@ edition = "2021" [dependencies] anyhow = "1" +askama = "0.13" clap = { version = "4.5", features = ["derive"] } csv = "1" diff = "0.1" diff --git a/src/ci/citool/src/main.rs b/src/ci/citool/src/main.rs index 0fee862f572..a7a289fc3d4 100644 --- a/src/ci/citool/src/main.rs +++ b/src/ci/citool/src/main.rs @@ -4,6 +4,7 @@ mod datadog; mod github; mod jobs; mod metrics; +mod test_dashboard; mod utils; use std::collections::{BTreeMap, HashMap}; @@ -22,6 +23,7 @@ use crate::datadog::upload_datadog_metric; use crate::github::JobInfoResolver; use crate::jobs::RunType; use crate::metrics::{JobMetrics, download_auto_job_metrics, download_job_metrics, load_metrics}; +use crate::test_dashboard::generate_test_dashboard; use crate::utils::load_env_var; const CI_DIRECTORY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/.."); @@ -234,6 +236,14 @@ enum Args { /// Current commit that will be compared to `parent`. current: String, }, + /// Generate a directory containing a HTML dashboard of test results from a CI run. + TestDashboard { + /// Commit SHA that was tested on CI to analyze. + current: String, + /// Output path for the HTML directory. + #[clap(long)] + output_dir: PathBuf, + }, } #[derive(clap::ValueEnum, Clone)] @@ -275,7 +285,11 @@ fn main() -> anyhow::Result<()> { postprocess_metrics(metrics_path, parent, job_name)?; } Args::PostMergeReport { current, parent } => { - post_merge_report(load_db(default_jobs_file)?, current, parent)?; + post_merge_report(load_db(&default_jobs_file)?, current, parent)?; + } + Args::TestDashboard { current, output_dir } => { + let db = load_db(&default_jobs_file)?; + generate_test_dashboard(db, ¤t, &output_dir)?; } } diff --git a/src/ci/citool/src/test_dashboard/mod.rs b/src/ci/citool/src/test_dashboard/mod.rs new file mode 100644 index 00000000000..ad9fe029e15 --- /dev/null +++ b/src/ci/citool/src/test_dashboard/mod.rs @@ -0,0 +1,239 @@ +use std::collections::{BTreeMap, HashMap}; +use std::fs::File; +use std::io::BufWriter; +use std::path::{Path, PathBuf}; + +use askama::Template; +use build_helper::metrics::{TestOutcome, TestSuiteMetadata}; + +use crate::jobs::JobDatabase; +use crate::metrics::{JobMetrics, JobName, download_auto_job_metrics, get_test_suites}; +use crate::utils::normalize_path_delimiters; + +pub struct TestInfo { + name: String, + jobs: Vec, +} + +struct JobTestResult { + job_name: String, + outcome: TestOutcome, +} + +#[derive(Default)] +struct TestSuiteInfo { + name: String, + tests: BTreeMap, +} + +/// Generate a set of HTML files into a directory that contain a dashboard of test results. +pub fn generate_test_dashboard( + db: JobDatabase, + current: &str, + output_dir: &Path, +) -> anyhow::Result<()> { + let metrics = download_auto_job_metrics(&db, None, current)?; + + let suites = gather_test_suites(&metrics); + + std::fs::create_dir_all(output_dir)?; + + let test_count = suites.test_count(); + write_page(output_dir, "index.html", &TestSuitesPage { suites, test_count })?; + + Ok(()) +} + +fn write_page(dir: &Path, name: &str, template: &T) -> anyhow::Result<()> { + let mut file = BufWriter::new(File::create(dir.join(name))?); + Template::write_into(template, &mut file)?; + Ok(()) +} + +fn gather_test_suites(job_metrics: &HashMap) -> TestSuites { + struct CoarseTestSuite<'a> { + kind: TestSuiteKind, + tests: BTreeMap>, + } + + let mut suites: HashMap = HashMap::new(); + + // First, gather tests from all jobs, stages and targets, and aggregate them per suite + for (job, metrics) in job_metrics { + let test_suites = get_test_suites(&metrics.current); + for suite in test_suites { + let (suite_name, stage, target, kind) = match &suite.metadata { + TestSuiteMetadata::CargoPackage { crates, stage, target, .. } => { + (crates.join(","), *stage, target, TestSuiteKind::Cargo) + } + TestSuiteMetadata::Compiletest { suite, stage, target, .. } => { + (suite.clone(), *stage, target, TestSuiteKind::Compiletest) + } + }; + let suite_entry = suites + .entry(suite_name.clone()) + .or_insert_with(|| CoarseTestSuite { kind, tests: Default::default() }); + let test_metadata = TestMetadata { job, stage, target }; + + for test in &suite.tests { + let test_name = normalize_test_name(&test.name, &suite_name); + let test_entry = suite_entry + .tests + .entry(test_name.clone()) + .or_insert_with(|| Test { name: test_name, passed: vec![], ignored: vec![] }); + match test.outcome { + TestOutcome::Passed => { + test_entry.passed.push(test_metadata); + } + TestOutcome::Ignored { ignore_reason: _ } => { + test_entry.ignored.push(test_metadata); + } + TestOutcome::Failed => { + eprintln!("Warning: failed test"); + } + } + } + } + } + + // Then, split the suites per directory + let mut suites = suites.into_iter().collect::>(); + suites.sort_by(|a, b| a.1.kind.cmp(&b.1.kind).then_with(|| a.0.cmp(&b.0))); + + let mut target_suites = vec![]; + for (suite_name, suite) in suites { + let suite = match suite.kind { + TestSuiteKind::Compiletest => TestSuite { + name: suite_name.clone(), + kind: TestSuiteKind::Compiletest, + group: build_test_group(&suite_name, suite.tests), + }, + TestSuiteKind::Cargo => { + let mut tests: Vec<_> = suite.tests.into_iter().collect(); + tests.sort_by(|a, b| a.0.cmp(&b.0)); + TestSuite { + name: format!("[cargo] {}", suite_name.clone()), + kind: TestSuiteKind::Cargo, + group: TestGroup { + name: suite_name, + root_tests: tests.into_iter().map(|t| t.1).collect(), + groups: vec![], + }, + } + } + }; + target_suites.push(suite); + } + + TestSuites { suites: target_suites } +} + +/// Recursively expand a test group based on filesystem hierarchy. +fn build_test_group<'a>(name: &str, tests: BTreeMap>) -> TestGroup<'a> { + let mut root_tests = vec![]; + let mut subdirs: BTreeMap>> = Default::default(); + + // Split tests into root tests and tests located in subdirectories + for (name, test) in tests { + let mut components = Path::new(&name).components().peekable(); + let subdir = components.next().unwrap(); + + if components.peek().is_none() { + // This is a root test + root_tests.push(test); + } else { + // This is a test in a nested directory + let subdir_tests = + subdirs.entry(subdir.as_os_str().to_str().unwrap().to_string()).or_default(); + let test_name = + components.into_iter().collect::().to_str().unwrap().to_string(); + subdir_tests.insert(test_name, test); + } + } + let dirs = subdirs + .into_iter() + .map(|(name, tests)| { + let group = build_test_group(&name, tests); + (name, group) + }) + .collect(); + + TestGroup { name: name.to_string(), root_tests, groups: dirs } +} + +/// Compiletest tests start with `[suite] tests/[suite]/a/b/c...`. +/// Remove the `[suite] tests/[suite]/` prefix so that we can find the filesystem path. +/// Also normalizes path delimiters. +fn normalize_test_name(name: &str, suite_name: &str) -> String { + let name = normalize_path_delimiters(name); + let name = name.as_ref(); + let name = name.strip_prefix(&format!("[{suite_name}]")).unwrap_or(name).trim(); + let name = name.strip_prefix("tests/").unwrap_or(name); + let name = name.strip_prefix(suite_name).unwrap_or(name); + name.trim_start_matches("/").to_string() +} + +#[derive(serde::Serialize)] +struct TestSuites<'a> { + suites: Vec>, +} + +impl<'a> TestSuites<'a> { + fn test_count(&self) -> u64 { + self.suites.iter().map(|suite| suite.group.test_count()).sum::() + } +} + +#[derive(serde::Serialize)] +struct TestSuite<'a> { + name: String, + kind: TestSuiteKind, + group: TestGroup<'a>, +} + +#[derive(Debug, serde::Serialize)] +struct Test<'a> { + name: String, + passed: Vec>, + ignored: Vec>, +} + +#[derive(Clone, Copy, Debug, serde::Serialize)] +struct TestMetadata<'a> { + job: &'a str, + stage: u32, + target: &'a str, +} + +// We have to use a template for the TestGroup instead of a macro, because +// macros cannot be recursive in askama at the moment. +#[derive(Template, serde::Serialize)] +#[template(path = "test_group.askama")] +/// Represents a group of tests +struct TestGroup<'a> { + name: String, + /// Tests located directly in this directory + root_tests: Vec>, + /// Nested directories with additional tests + groups: Vec<(String, TestGroup<'a>)>, +} + +impl<'a> TestGroup<'a> { + fn test_count(&self) -> u64 { + let root = self.root_tests.len() as u64; + self.groups.iter().map(|(_, group)| group.test_count()).sum::() + root + } +} + +#[derive(PartialEq, Eq, PartialOrd, Ord, serde::Serialize)] +enum TestSuiteKind { + Compiletest, + Cargo, +} + +#[derive(Template)] +#[template(path = "test_suites.askama")] +struct TestSuitesPage<'a> { + suites: TestSuites<'a>, + test_count: u64, +} diff --git a/src/ci/citool/templates/layout.askama b/src/ci/citool/templates/layout.askama new file mode 100644 index 00000000000..2e830aaa9f5 --- /dev/null +++ b/src/ci/citool/templates/layout.askama @@ -0,0 +1,71 @@ + + + + Rust CI Test Dashboard + + + + +{% block content %}{% endblock %} + + diff --git a/src/ci/citool/templates/test_group.askama b/src/ci/citool/templates/test_group.askama new file mode 100644 index 00000000000..a0b7fa863e5 --- /dev/null +++ b/src/ci/citool/templates/test_group.askama @@ -0,0 +1,22 @@ +
  • +
    +{{ name }} ({{ test_count() }} test{{ test_count() | pluralize }}) + +{% if !groups.is_empty() %} +
      + {% for (dir_name, subgroup) in groups %} + {{ subgroup|safe }} + {% endfor %} +
    +{% endif %} + +{% if !root_tests.is_empty() %} +
      + {% for test in root_tests %} +
    • {{ test.name }} ({{ test.passed.len() }} passed, {{ test.ignored.len() }} ignored)
      • + {% endfor %} +
    +{% endif %} + +
    +
    • diff --git a/src/ci/citool/templates/test_suites.askama b/src/ci/citool/templates/test_suites.askama new file mode 100644 index 00000000000..a6f8d0e1abe --- /dev/null +++ b/src/ci/citool/templates/test_suites.askama @@ -0,0 +1,18 @@ +{% extends "layout.askama" %} + +{% block content %} +

    Rust CI Test Dashboard

    +
    +
    + Total tests: {{ test_count }} +
    + +
      + {% for suite in suites.suites %} + {% if suite.kind == TestSuiteKind::Compiletest %} + {{ suite.group|safe }} + {% endif %} + {% endfor %} +
    +
    +{% endblock %} From a326afd5dd810427c72ed81e705c0d903e74edcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 16:30:23 +0200 Subject: [PATCH 30/67] Add buttons for expanding and collapsing all test suites --- src/ci/citool/templates/layout.askama | 57 ++------------- src/ci/citool/templates/test_suites.askama | 81 +++++++++++++++++++++- 2 files changed, 84 insertions(+), 54 deletions(-) diff --git a/src/ci/citool/templates/layout.askama b/src/ci/citool/templates/layout.askama index 2e830aaa9f5..3b3b6f23741 100644 --- a/src/ci/citool/templates/layout.askama +++ b/src/ci/citool/templates/layout.askama @@ -3,69 +3,20 @@ Rust CI Test Dashboard {% block content %}{% endblock %} +{% block scripts %}{% endblock %} diff --git a/src/ci/citool/templates/test_suites.askama b/src/ci/citool/templates/test_suites.askama index a6f8d0e1abe..a8cedc65f24 100644 --- a/src/ci/citool/templates/test_suites.askama +++ b/src/ci/citool/templates/test_suites.askama @@ -4,7 +4,11 @@

    Rust CI Test Dashboard

    - Total tests: {{ test_count }} + Total tests: {{ test_count }} +
    + + +
      @@ -16,3 +20,78 @@
    {% endblock %} + +{% block styles %} +h1 { + text-align: center; + color: #333333; + margin-bottom: 30px; +} + +.summary { + display: flex; + justify-content: space-between; +} + +.test-suites { + background: white; + border-radius: 8px; + box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1); + padding: 20px; +} + +ul { + padding-left: 0; +} + +li { + list-style: none; + padding-left: 20px; +} +summary { + margin-bottom: 5px; + padding: 6px; + background-color: #F4F4F4; + border: 1px solid #ddd; + border-radius: 4px; + cursor: pointer; +} +summary:hover { + background-color: #CFCFCF; +} + +/* Style the disclosure triangles */ +details > summary { + list-style: none; + position: relative; +} + +details > summary::before { + content: "▶"; + position: absolute; + left: -15px; + transform: rotate(0); + transition: transform 0.2s; +} + +details[open] > summary::before { + transform: rotate(90deg); +} +{% endblock %} + +{% block scripts %} + +{% endblock %} From 4b310338f8d2a67cbc863ee799206709e95da6b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 16:35:34 +0200 Subject: [PATCH 31/67] Add a note about how to find tests that haven't been executed anywhere. --- src/ci/citool/src/test_dashboard/mod.rs | 60 ++++------------------ src/ci/citool/templates/test_suites.askama | 17 ++++-- 2 files changed, 22 insertions(+), 55 deletions(-) diff --git a/src/ci/citool/src/test_dashboard/mod.rs b/src/ci/citool/src/test_dashboard/mod.rs index ad9fe029e15..163e9c1acea 100644 --- a/src/ci/citool/src/test_dashboard/mod.rs +++ b/src/ci/citool/src/test_dashboard/mod.rs @@ -10,22 +10,6 @@ use crate::jobs::JobDatabase; use crate::metrics::{JobMetrics, JobName, download_auto_job_metrics, get_test_suites}; use crate::utils::normalize_path_delimiters; -pub struct TestInfo { - name: String, - jobs: Vec, -} - -struct JobTestResult { - job_name: String, - outcome: TestOutcome, -} - -#[derive(Default)] -struct TestSuiteInfo { - name: String, - tests: BTreeMap, -} - /// Generate a set of HTML files into a directory that contain a dashboard of test results. pub fn generate_test_dashboard( db: JobDatabase, @@ -33,7 +17,6 @@ pub fn generate_test_dashboard( output_dir: &Path, ) -> anyhow::Result<()> { let metrics = download_auto_job_metrics(&db, None, current)?; - let suites = gather_test_suites(&metrics); std::fs::create_dir_all(output_dir)?; @@ -52,27 +35,27 @@ fn write_page(dir: &Path, name: &str, template: &T) -> anyhow::Resu fn gather_test_suites(job_metrics: &HashMap) -> TestSuites { struct CoarseTestSuite<'a> { - kind: TestSuiteKind, tests: BTreeMap>, } let mut suites: HashMap = HashMap::new(); // First, gather tests from all jobs, stages and targets, and aggregate them per suite + // Only work with compiletest suites. for (job, metrics) in job_metrics { let test_suites = get_test_suites(&metrics.current); for suite in test_suites { - let (suite_name, stage, target, kind) = match &suite.metadata { - TestSuiteMetadata::CargoPackage { crates, stage, target, .. } => { - (crates.join(","), *stage, target, TestSuiteKind::Cargo) + let (suite_name, stage, target) = match &suite.metadata { + TestSuiteMetadata::CargoPackage { .. } => { + continue; } TestSuiteMetadata::Compiletest { suite, stage, target, .. } => { - (suite.clone(), *stage, target, TestSuiteKind::Compiletest) + (suite.clone(), *stage, target) } }; let suite_entry = suites .entry(suite_name.clone()) - .or_insert_with(|| CoarseTestSuite { kind, tests: Default::default() }); + .or_insert_with(|| CoarseTestSuite { tests: Default::default() }); let test_metadata = TestMetadata { job, stage, target }; for test in &suite.tests { @@ -98,29 +81,13 @@ fn gather_test_suites(job_metrics: &HashMap) -> TestSuites // Then, split the suites per directory let mut suites = suites.into_iter().collect::>(); - suites.sort_by(|a, b| a.1.kind.cmp(&b.1.kind).then_with(|| a.0.cmp(&b.0))); + suites.sort_by(|a, b| a.0.cmp(&b.0)); let mut target_suites = vec![]; for (suite_name, suite) in suites { - let suite = match suite.kind { - TestSuiteKind::Compiletest => TestSuite { - name: suite_name.clone(), - kind: TestSuiteKind::Compiletest, - group: build_test_group(&suite_name, suite.tests), - }, - TestSuiteKind::Cargo => { - let mut tests: Vec<_> = suite.tests.into_iter().collect(); - tests.sort_by(|a, b| a.0.cmp(&b.0)); - TestSuite { - name: format!("[cargo] {}", suite_name.clone()), - kind: TestSuiteKind::Cargo, - group: TestGroup { - name: suite_name, - root_tests: tests.into_iter().map(|t| t.1).collect(), - groups: vec![], - }, - } - } + let suite = TestSuite { + name: suite_name.clone(), + group: build_test_group(&suite_name, suite.tests), }; target_suites.push(suite); } @@ -187,7 +154,6 @@ impl<'a> TestSuites<'a> { #[derive(serde::Serialize)] struct TestSuite<'a> { name: String, - kind: TestSuiteKind, group: TestGroup<'a>, } @@ -225,12 +191,6 @@ impl<'a> TestGroup<'a> { } } -#[derive(PartialEq, Eq, PartialOrd, Ord, serde::Serialize)] -enum TestSuiteKind { - Compiletest, - Cargo, -} - #[derive(Template)] #[template(path = "test_suites.askama")] struct TestSuitesPage<'a> { diff --git a/src/ci/citool/templates/test_suites.askama b/src/ci/citool/templates/test_suites.askama index a8cedc65f24..bb3d9e36391 100644 --- a/src/ci/citool/templates/test_suites.askama +++ b/src/ci/citool/templates/test_suites.askama @@ -1,10 +1,15 @@ {% extends "layout.askama" %} {% block content %} -

    Rust CI Test Dashboard

    +

    Rust CI test dashboard

    - Total tests: {{ test_count }} +
    +
    Total tests: {{ test_count }}
    +
    + To find tests that haven't been executed anywhere, click on "Open all" and search for "(0 passed". +
    +
    @@ -13,9 +18,7 @@
      {% for suite in suites.suites %} - {% if suite.kind == TestSuiteKind::Compiletest %} - {{ suite.group|safe }} - {% endif %} + {{ suite.group|safe }} {% endfor %}
    @@ -33,6 +36,10 @@ h1 { justify-content: space-between; } +.test-count { + font-size: 1.2em; +} + .test-suites { background: white; border-radius: 8px; From 1a6e0d52e5b008cfd48f78285bb3655ecfd5d73e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 17:14:26 +0200 Subject: [PATCH 32/67] Render test revisions separately --- src/ci/citool/src/test_dashboard/mod.rs | 43 ++++++++++++++++++----- src/ci/citool/templates/test_group.askama | 13 ++++++- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/ci/citool/src/test_dashboard/mod.rs b/src/ci/citool/src/test_dashboard/mod.rs index 163e9c1acea..c16385baa3b 100644 --- a/src/ci/citool/src/test_dashboard/mod.rs +++ b/src/ci/citool/src/test_dashboard/mod.rs @@ -60,19 +60,27 @@ fn gather_test_suites(job_metrics: &HashMap) -> TestSuites for test in &suite.tests { let test_name = normalize_test_name(&test.name, &suite_name); - let test_entry = suite_entry - .tests - .entry(test_name.clone()) - .or_insert_with(|| Test { name: test_name, passed: vec![], ignored: vec![] }); + let (test_name, variant_name) = match test_name.rsplit_once('#') { + Some((name, variant)) => (name.to_string(), variant.to_string()), + None => (test_name, "".to_string()), + }; + let test_entry = suite_entry.tests.entry(test_name.clone()).or_insert_with(|| { + Test { name: test_name.clone(), revisions: Default::default() } + }); + let variant_entry = test_entry + .revisions + .entry(variant_name) + .or_insert_with(|| TestResults { passed: vec![], ignored: vec![] }); + match test.outcome { TestOutcome::Passed => { - test_entry.passed.push(test_metadata); + variant_entry.passed.push(test_metadata); } TestOutcome::Ignored { ignore_reason: _ } => { - test_entry.ignored.push(test_metadata); + variant_entry.ignored.push(test_metadata); } TestOutcome::Failed => { - eprintln!("Warning: failed test"); + eprintln!("Warning: failed test {test_name}"); } } } @@ -158,12 +166,29 @@ struct TestSuite<'a> { } #[derive(Debug, serde::Serialize)] -struct Test<'a> { - name: String, +struct TestResults<'a> { passed: Vec>, ignored: Vec>, } +#[derive(Debug, serde::Serialize)] +struct Test<'a> { + name: String, + revisions: BTreeMap>, +} + +impl<'a> Test<'a> { + /// If this is a test without revisions, it will have a single entry in `revisions` with + /// an empty string as the revision name. + fn single_test(&self) -> Option<&TestResults<'a>> { + if self.revisions.len() == 1 { + self.revisions.iter().next().take_if(|e| e.0.is_empty()).map(|e| e.1) + } else { + None + } + } +} + #[derive(Clone, Copy, Debug, serde::Serialize)] struct TestMetadata<'a> { job: &'a str, diff --git a/src/ci/citool/templates/test_group.askama b/src/ci/citool/templates/test_group.askama index a0b7fa863e5..535d98e0c24 100644 --- a/src/ci/citool/templates/test_group.askama +++ b/src/ci/citool/templates/test_group.askama @@ -13,7 +13,18 @@ {% if !root_tests.is_empty() %}
      {% for test in root_tests %} -
    • {{ test.name }} ({{ test.passed.len() }} passed, {{ test.ignored.len() }} ignored)
      • +
    • + {% if let Some(result) = test.single_test() %} + {{ test.name }} ({{ result.passed.len() }} passed, {{ result.ignored.len() }} ignored) + {% else %} + {{ test.name }} ({{ test.revisions.len() }} revision{{ test.revisions.len() | pluralize }}) +
        + {% for (revision, result) in test.revisions %} +
      • #{{ revision }} ({{ result.passed.len() }} passed, {{ result.ignored.len() }} ignored)
        • + {% endfor %} +
      + {% endif %} +
      • {% endfor %}
    {% endif %} From d2c1763336080030a235dae56f6096e9deb2ec9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 17:18:38 +0200 Subject: [PATCH 33/67] Create a macro for rendering test results --- src/ci/citool/templates/test_group.askama | 8 ++++++-- src/ci/citool/templates/test_suites.askama | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/ci/citool/templates/test_group.askama b/src/ci/citool/templates/test_group.askama index 535d98e0c24..ba19d9258d8 100644 --- a/src/ci/citool/templates/test_group.askama +++ b/src/ci/citool/templates/test_group.askama @@ -1,3 +1,7 @@ +{% macro test_result(r) -%} +passed: {{ r.passed.len() }}, ignored: {{ r.ignored.len() }} +{%- endmacro %} +
  • {{ name }} ({{ test_count() }} test{{ test_count() | pluralize }}) @@ -15,12 +19,12 @@ {% for test in root_tests %}
  • {% if let Some(result) = test.single_test() %} - {{ test.name }} ({{ result.passed.len() }} passed, {{ result.ignored.len() }} ignored) + {{ test.name }} ({% call test_result(result) %}) {% else %} {{ test.name }} ({{ test.revisions.len() }} revision{{ test.revisions.len() | pluralize }})
      {% for (revision, result) in test.revisions %} -
    • #{{ revision }} ({{ result.passed.len() }} passed, {{ result.ignored.len() }} ignored)
      • +
    • #{{ revision }} ({% call test_result(result) %})
      • {% endfor %}
    {% endif %} diff --git a/src/ci/citool/templates/test_suites.askama b/src/ci/citool/templates/test_suites.askama index bb3d9e36391..d36e85228e2 100644 --- a/src/ci/citool/templates/test_suites.askama +++ b/src/ci/citool/templates/test_suites.askama @@ -7,7 +7,7 @@
    Total tests: {{ test_count }}
    - To find tests that haven't been executed anywhere, click on "Open all" and search for "(0 passed". + To find tests that haven't been executed anywhere, click on "Open all" and search for "passed: 0".
    From aa9cb70190bb38e466983abf0c53c6f26afab4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 17:25:12 +0200 Subject: [PATCH 34/67] Print number of root tests and subdirectories --- src/ci/citool/templates/test_group.askama | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ci/citool/templates/test_group.askama b/src/ci/citool/templates/test_group.askama index ba19d9258d8..bdf32d00f4a 100644 --- a/src/ci/citool/templates/test_group.askama +++ b/src/ci/citool/templates/test_group.askama @@ -4,7 +4,12 @@ passed: {{ r.passed.len() }}, ignored: {{ r.ignored.len() }}
  • -{{ name }} ({{ test_count() }} test{{ test_count() | pluralize }}) +{{ name }} ({{ test_count() }} test{{ test_count() | pluralize }}{% if !root_tests.is_empty() && root_tests.len() as u64 != test_count() -%} + , {{ root_tests.len() }} root test{{ root_tests.len() | pluralize }} +{%- endif %}{% if !groups.is_empty() -%} + , {{ groups.len() }} subdir{{ groups.len() | pluralize }} +{%- endif %}) + {% if !groups.is_empty() %}