From 09ae438eb095e6e2a94e1516cb962e66bfc6d747 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 8 Aug 2024 10:20:26 +0200 Subject: [PATCH 01/13] Add `Steal::is_stolen()` --- compiler/rustc_data_structures/src/steal.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/compiler/rustc_data_structures/src/steal.rs b/compiler/rustc_data_structures/src/steal.rs index 9a0fd52677d13..f305a8ad12000 100644 --- a/compiler/rustc_data_structures/src/steal.rs +++ b/compiler/rustc_data_structures/src/steal.rs @@ -51,6 +51,12 @@ impl Steal { let value = value_ref.take(); value.expect("attempt to steal from stolen value") } + + /// Writers of rustc drivers often encounter stealing issues. This function makes it possible to + /// handle these errors gracefully. This is not used within rustc as the time of writing. + pub fn is_stolen(&self) -> bool { + self.value.borrow().is_none() + } } impl> HashStable for Steal { From c966370b1959707be06dd2cbfb1436233fa595a3 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 8 Aug 2024 21:51:50 +0200 Subject: [PATCH 02/13] Tweak wording Co-authored-by: lcnr --- compiler/rustc_data_structures/src/steal.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_data_structures/src/steal.rs b/compiler/rustc_data_structures/src/steal.rs index f305a8ad12000..0f2c0eee27d2f 100644 --- a/compiler/rustc_data_structures/src/steal.rs +++ b/compiler/rustc_data_structures/src/steal.rs @@ -53,7 +53,10 @@ impl Steal { } /// Writers of rustc drivers often encounter stealing issues. This function makes it possible to - /// handle these errors gracefully. This is not used within rustc as the time of writing. + /// handle these errors gracefully. + /// + /// This should not be used within rustc as it leaks information not tracked + /// by the query system, breaking incremental compilation. pub fn is_stolen(&self) -> bool { self.value.borrow().is_none() } From 11b801bc4e410e77e6935a0c4544d4a90b868214 Mon Sep 17 00:00:00 2001 From: Min <45393763+MinxuanZ@users.noreply.github.com> Date: Thu, 8 Aug 2024 15:36:58 +0800 Subject: [PATCH 03/13] [MIPS] fix the name of signal 19 in mips --- library/std/src/sys/pal/unix/process/process_unix/tests.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/std/src/sys/pal/unix/process/process_unix/tests.rs b/library/std/src/sys/pal/unix/process/process_unix/tests.rs index e5e1f956bc351..1ca1ae7f3f45e 100644 --- a/library/std/src/sys/pal/unix/process/process_unix/tests.rs +++ b/library/std/src/sys/pal/unix/process/process_unix/tests.rs @@ -24,6 +24,9 @@ fn exitstatus_display_tests() { // The purpose of this test is to test our string formatting, not our understanding of the wait // status magic numbers. So restrict these to Linux. if cfg!(target_os = "linux") { + #[cfg(any(target_arch = "mips", target_arch = "mips64"))] + t(0x0137f, "stopped (not terminated) by signal: 19 (SIGPWR)"); + #[cfg(not(any(target_arch = "mips", target_arch = "mips64")))] t(0x0137f, "stopped (not terminated) by signal: 19 (SIGSTOP)"); t(0x0ffff, "continued (WIFCONTINUED)"); } From a3f8edff20a7d8aadd98d2553bd9ad12508a9980 Mon Sep 17 00:00:00 2001 From: Min <45393763+MinxuanZ@users.noreply.github.com> Date: Thu, 8 Aug 2024 15:53:53 +0800 Subject: [PATCH 04/13] [SPARC] fix the name of signal 19 in sparc arch --- .../std/src/sys/pal/unix/process/process_unix/tests.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/pal/unix/process/process_unix/tests.rs b/library/std/src/sys/pal/unix/process/process_unix/tests.rs index 1ca1ae7f3f45e..f6044c4455c27 100644 --- a/library/std/src/sys/pal/unix/process/process_unix/tests.rs +++ b/library/std/src/sys/pal/unix/process/process_unix/tests.rs @@ -26,8 +26,16 @@ fn exitstatus_display_tests() { if cfg!(target_os = "linux") { #[cfg(any(target_arch = "mips", target_arch = "mips64"))] t(0x0137f, "stopped (not terminated) by signal: 19 (SIGPWR)"); - #[cfg(not(any(target_arch = "mips", target_arch = "mips64")))] + + #[cfg(any(target_arch = "sparc", target_arch = "sparc64"))] + t(0x0137f, "stopped (not terminated) by signal: 19 (SIGCONT)"); + + #[cfg(not(any(target_arch = "mips", + target_arch = "sparc", + target_arch = "mips64", + target_arch = "sprac64")))] t(0x0137f, "stopped (not terminated) by signal: 19 (SIGSTOP)"); + t(0x0ffff, "continued (WIFCONTINUED)"); } From 625432c837875e40b4167c0a5c7702b2f3c671a0 Mon Sep 17 00:00:00 2001 From: monstercatss Date: Fri, 9 Aug 2024 09:36:22 +0800 Subject: [PATCH 05/13] fix format --- .../std/src/sys/pal/unix/process/process_unix/tests.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/pal/unix/process/process_unix/tests.rs b/library/std/src/sys/pal/unix/process/process_unix/tests.rs index f6044c4455c27..9410b1a87da0a 100644 --- a/library/std/src/sys/pal/unix/process/process_unix/tests.rs +++ b/library/std/src/sys/pal/unix/process/process_unix/tests.rs @@ -30,10 +30,12 @@ fn exitstatus_display_tests() { #[cfg(any(target_arch = "sparc", target_arch = "sparc64"))] t(0x0137f, "stopped (not terminated) by signal: 19 (SIGCONT)"); - #[cfg(not(any(target_arch = "mips", - target_arch = "sparc", - target_arch = "mips64", - target_arch = "sprac64")))] + #[cfg(not(any( + target_arch = "mips", + target_arch = "mips64", + target_arch = "sparc", + target_arch = "sparc64" + )))] t(0x0137f, "stopped (not terminated) by signal: 19 (SIGSTOP)"); t(0x0ffff, "continued (WIFCONTINUED)"); From 0106f5bcbab8ded01bc4b027d8381d2a3a7cc265 Mon Sep 17 00:00:00 2001 From: monstercatss Date: Fri, 9 Aug 2024 10:12:54 +0800 Subject: [PATCH 06/13] delete space --- library/std/src/sys/pal/unix/process/process_unix/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/pal/unix/process/process_unix/tests.rs b/library/std/src/sys/pal/unix/process/process_unix/tests.rs index 9410b1a87da0a..f4d6ac6b4e340 100644 --- a/library/std/src/sys/pal/unix/process/process_unix/tests.rs +++ b/library/std/src/sys/pal/unix/process/process_unix/tests.rs @@ -31,13 +31,13 @@ fn exitstatus_display_tests() { t(0x0137f, "stopped (not terminated) by signal: 19 (SIGCONT)"); #[cfg(not(any( - target_arch = "mips", + target_arch = "mips", target_arch = "mips64", target_arch = "sparc", target_arch = "sparc64" )))] t(0x0137f, "stopped (not terminated) by signal: 19 (SIGSTOP)"); - + t(0x0ffff, "continued (WIFCONTINUED)"); } From b589f86a09c823208cbe95755f0cb0883e28d0de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Fri, 9 Aug 2024 05:03:36 +0000 Subject: [PATCH 07/13] tests: add regression test for incorrect `BytePos` manipulation triggering assertion Issue: --- .../ui/typeck/suggest-arg-comma-delete-ice.rs | 19 ++++++++++ .../suggest-arg-comma-delete-ice.stderr | 38 +++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 tests/ui/typeck/suggest-arg-comma-delete-ice.rs create mode 100644 tests/ui/typeck/suggest-arg-comma-delete-ice.stderr diff --git a/tests/ui/typeck/suggest-arg-comma-delete-ice.rs b/tests/ui/typeck/suggest-arg-comma-delete-ice.rs new file mode 100644 index 0000000000000..48d02e13eca09 --- /dev/null +++ b/tests/ui/typeck/suggest-arg-comma-delete-ice.rs @@ -0,0 +1,19 @@ +//! Previously, we tried to remove extra arg commas when providing extra arg removal suggestions. +//! One of the edge cases is having to account for an arg that has a closing delimiter `)` +//! following it. However, the previous suggestion code assumed that the delimiter is in fact +//! exactly the 1-byte `)` character. This assumption was proven incorrect, because we recover +//! from Unicode-confusable delimiters in the parser, which means that the ending delimiter could be +//! a multi-byte codepoint that looks *like* a `)`. Subtracing 1 byte could land us in the middle of +//! a codepoint, triggering a codepoint boundary assertion. +//! +//! issue: rust-lang/rust#128717 + +fn main() { + // The following example has been modified from #128717 to remove irrelevant Unicode as they do + // not otherwise partake in the right delimiter calculation causing the codepoint boundary + // assertion. + main(rahh）; + //~^ ERROR unknown start of token + //~| ERROR this function takes 0 arguments but 1 argument was supplied + //~| ERROR cannot find value `rahh` in this scope +} diff --git a/tests/ui/typeck/suggest-arg-comma-delete-ice.stderr b/tests/ui/typeck/suggest-arg-comma-delete-ice.stderr new file mode 100644 index 0000000000000..53608391f3c89 --- /dev/null +++ b/tests/ui/typeck/suggest-arg-comma-delete-ice.stderr @@ -0,0 +1,38 @@ +error: unknown start of token: \u{ff09} + --> $DIR/suggest-arg-comma-delete-ice.rs:15:14 + | +LL | main(rahh）; + | ^^ + | +help: Unicode character '）' (Fullwidth Right Parenthesis) looks like ')' (Right Parenthesis), but it is not + | +LL | main(rahh); + | ~ + +error[E0425]: cannot find value `rahh` in this scope + --> $DIR/suggest-arg-comma-delete-ice.rs:15:10 + | +LL | main(rahh）; + | ^^^^ not found in this scope + +error[E0061]: this function takes 0 arguments but 1 argument was supplied + --> $DIR/suggest-arg-comma-delete-ice.rs:15:5 + | +LL | main(rahh）; + | ^^^^ ---- unexpected argument + | +note: function defined here + --> $DIR/suggest-arg-comma-delete-ice.rs:11:4 + | +LL | fn main() { + | ^^^^ +help: remove the extra argument + | +LL - main(rahh）; +LL + main(）; + | + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0061, E0425. +For more information about an error, try `rustc --explain E0061`. From 879bfd7ad0f5f79e7bc90320dfb80dfabe91ac2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Fri, 9 Aug 2024 05:01:27 +0000 Subject: [PATCH 08/13] hir_typeck: use `end_point` over `BytePos` manipulations Parser has error recovery for Unicode-confusables, which includes the right parentheses `)`. If a multi-byte right parentheses look-alike reaches the argument removal suggestion diagnostics, it would trigger an assertion because the diagnostics used `- BytePos(1)` which can land within a multi-byte codepoint. This is fixed by using `SourceMap::end_point` to find the final right delimiter codepoint, which correctly respects codepoint boundaries. --- compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index cef003e0a43de..89e7227eda2c7 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -22,7 +22,7 @@ use rustc_middle::ty::{self, IsSuggestable, Ty, TyCtxt}; use rustc_middle::{bug, span_bug}; use rustc_session::Session; use rustc_span::symbol::{kw, Ident}; -use rustc_span::{sym, BytePos, Span, DUMMY_SP}; +use rustc_span::{sym, Span, DUMMY_SP}; use rustc_trait_selection::error_reporting::infer::{FailureCode, ObligationCauseExt}; use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::{self, ObligationCauseCode, SelectionContext}; @@ -1140,8 +1140,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .get(arg_idx + 1) .map(|&(_, sp)| sp) .unwrap_or_else(|| { - // Subtract one to move before `)` - call_expr.span.with_lo(call_expr.span.hi() - BytePos(1)) + // Try to move before `)`. Note that `)` here is not necessarily + // the latin right paren, it could be a Unicode-confusable that + // looks like a `)`, so we must not use `- BytePos(1)` + // manipulations here. + self.tcx().sess.source_map().end_point(call_expr.span) }); // Include next comma From 92520a9d4d74d23b29bd65f3372f1d0e5a0ec9cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Fri, 9 Aug 2024 05:48:58 +0000 Subject: [PATCH 09/13] tests: add regression test for #128845 For codepoint boundary assertion triggered by a let stmt compound assignment removal suggestion when encountering recovered multi-byte compound ops. Issue: --- .../suggest-remove-compount-assign-let-ice.rs | 16 ++++++++++++ ...gest-remove-compount-assign-let-ice.stderr | 26 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/ui/parser/suggest-remove-compount-assign-let-ice.rs create mode 100644 tests/ui/parser/suggest-remove-compount-assign-let-ice.stderr diff --git a/tests/ui/parser/suggest-remove-compount-assign-let-ice.rs b/tests/ui/parser/suggest-remove-compount-assign-let-ice.rs new file mode 100644 index 0000000000000..1affee5678e44 --- /dev/null +++ b/tests/ui/parser/suggest-remove-compount-assign-let-ice.rs @@ -0,0 +1,16 @@ +//! Previously we would try to issue a suggestion for `let x = 1`, i.e. a compound assignment +//! within a `let` binding, to remove the ``. The suggestion code unfortunately incorrectly +//! assumed that the `` is an exactly-1-byte ASCII character, but this assumption is incorrect +//! because we also recover Unicode-confusables like `➖=` as `-=`. In this example, the suggestion +//! code used a `+ BytePos(1)` to calculate the span of the `` codepoint that looks like `-` but +//! the mult-byte Unicode look-alike would cause the suggested removal span to be inside a +//! multi-byte codepoint boundary, triggering a codepoint boundary assertion. +//! +//! issue: rust-lang/rust#128845 + +fn main() { + // Adapted from #128845 but with irrelevant components removed and simplified. + let x ➖= 1; + //~^ ERROR unknown start of token: \u{2796} + //~| ERROR: can't reassign to an uninitialized variable +} diff --git a/tests/ui/parser/suggest-remove-compount-assign-let-ice.stderr b/tests/ui/parser/suggest-remove-compount-assign-let-ice.stderr new file mode 100644 index 0000000000000..59716d69b50f9 --- /dev/null +++ b/tests/ui/parser/suggest-remove-compount-assign-let-ice.stderr @@ -0,0 +1,26 @@ +error: unknown start of token: \u{2796} + --> $DIR/suggest-remove-compount-assign-let-ice.rs:13:11 + | +LL | let x ➖= 1; + | ^^ + | +help: Unicode character '➖' (Heavy Minus Sign) looks like '-' (Minus/Hyphen), but it is not + | +LL | let x -= 1; + | ~ + +error: can't reassign to an uninitialized variable + --> $DIR/suggest-remove-compount-assign-let-ice.rs:13:11 + | +LL | let x ➖= 1; + | ^^^ + | + = help: if you meant to overwrite, remove the `let` binding +help: initialize the variable + | +LL - let x ➖= 1; +LL + let x = 1; + | + +error: aborting due to 2 previous errors + From d65f1316bba2b3cdccce3f0d7d3e2eb27b5fbe49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Fri, 9 Aug 2024 05:48:52 +0000 Subject: [PATCH 10/13] parser: ensure let stmt compound assignment removal suggestion respect codepoint boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously we would try to issue a suggestion for `let x = 1`, i.e. a compound assignment within a `let` binding, to remove the ``. The suggestion code unfortunately incorrectly assumed that the `` is an exactly-1-byte ASCII character, but this assumption is incorrect because we also recover Unicode-confusables like `➖=` as `-=`. In this example, the suggestion code used a `+ BytePos(1)` to calculate the span of the `` codepoint that looks like `-` but the mult-byte Unicode look-alike would cause the suggested removal span to be inside a multi-byte codepoint boundary, triggering a codepoint boundary assertion. Issue: --- compiler/rustc_parse/src/parser/stmt.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index b3efb87a4a26a..a3b782d651d3f 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -408,10 +408,14 @@ impl<'a> Parser<'a> { fn parse_initializer(&mut self, eq_optional: bool) -> PResult<'a, Option>> { let eq_consumed = match self.token.kind { token::BinOpEq(..) => { - // Recover `let x = 1` as `let x = 1` + // Recover `let x = 1` as `let x = 1` We must not use `+ BytePos(1)` here + // because `` can be a multi-byte lookalike that was recovered, e.g. `➖=` (the + // `➖` is a U+2796 Heavy Minus Sign Unicode Character) that was recovered as a + // `-=`. + let extra_op_span = self.psess.source_map().start_point(self.token.span); self.dcx().emit_err(errors::CompoundAssignmentExpressionInLet { span: self.token.span, - suggestion: self.token.span.with_hi(self.token.span.lo() + BytePos(1)), + suggestion: extra_op_span, }); self.bump(); true From 38874a692738f23b17d20a81bb906701303c0474 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 8 Aug 2024 23:55:59 +0200 Subject: [PATCH 11/13] use stable sort to sort multipart diagnostics --- compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs | 2 +- compiler/rustc_errors/src/diagnostic.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 7a925705806fc..a58c7c43246b5 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1124,8 +1124,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { err.multipart_suggestion( "consider moving the expression out of the loop so it is only moved once", vec![ - (parent.span, "value".to_string()), (span.shrink_to_lo(), format!("let mut value = {value};{indent}")), + (parent.span, "value".to_string()), ], Applicability::MaybeIncorrect, ); diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 67ca6d50cca4b..fae8b5647fc9a 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -920,8 +920,8 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { applicability: Applicability, style: SuggestionStyle, ) -> &mut Self { - suggestion.sort_unstable(); - suggestion.dedup_by(|(s1, m1), (s2, m2)| s1.source_equal(*s2) && m1 == m2); + let mut seen = crate::FxHashSet::default(); + suggestion.retain(|(span, msg)| seen.insert((span.lo(), span.hi(), msg.clone()))); let parts = suggestion .into_iter() From f72cb0415bdb6fe3699fb44f9b41da1d34c74189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 9 Aug 2024 12:17:01 +0200 Subject: [PATCH 12/13] Make `Build::run` comment more accurate --- src/bootstrap/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 453fb39327d63..d3aff8f89a0ca 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -986,7 +986,8 @@ impl Build { } /// Execute a command and return its output. - /// This method should be used for all command executions in bootstrap. + /// Note: Ideally, you should use one of the BootstrapCommand::run* functions to + /// execute commands. They internally call this method. #[track_caller] fn run( &self, From a380d5e8f68de64441274ca2e56dd3a08a60a121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 9 Aug 2024 12:17:22 +0200 Subject: [PATCH 13/13] Do not print verbose error when a bootstrap command fails without verbose mode --- src/bootstrap/src/lib.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index d3aff8f89a0ca..2062d435bfc99 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -1058,20 +1058,28 @@ Executed at: {executed_at}"#, CommandOutput::did_not_start(stdout, stderr) } }; + + let fail = |message: &str| { + if self.is_verbose() { + println!("{message}"); + } else { + println!("Command has failed. Rerun with -v to see more details."); + } + exit!(1); + }; + if !output.is_success() { match command.failure_behavior { BehaviorOnFailure::DelayFail => { if self.fail_fast { - println!("{message}"); - exit!(1); + fail(&message); } let mut failures = self.delayed_failures.borrow_mut(); failures.push(message); } BehaviorOnFailure::Exit => { - println!("{message}"); - exit!(1); + fail(&message); } BehaviorOnFailure::Ignore => { // If failures are allowed, either the error has been printed already