From f1d594eb9e3dd85207e0264abad9b8674cbb8be2 Mon Sep 17 00:00:00 2001 From: Chris Wailes Date: Fri, 16 Feb 2024 14:44:41 -0800 Subject: [PATCH 01/32] Add `-Z external-clangrt` This adds the unstable `-Z external-clangrt` flag that will prevent rustc from emitting linker paths for the in-tree LLVM sanitizer runtime library. --- compiler/rustc_codegen_ssa/src/back/link.rs | 1 + compiler/rustc_session/src/options.rs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 000b2748e4f93..30a7434d12f1c 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1192,6 +1192,7 @@ fn add_sanitizer_libraries(sess: &Session, crate_type: CrateType, linker: &mut d // are currently distributed as static libraries which should be linked to // executables only. let needs_runtime = !sess.target.is_like_android + && !sess.opts.unstable_opts.external_clangrt && match crate_type { CrateType::Executable => true, CrateType::Dylib | CrateType::Cdylib | CrateType::ProcMacro => sess.target.is_like_osx, diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index e8ca556aa420d..cada28334e300 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1621,6 +1621,8 @@ options! { "emit the bc module with thin LTO info (default: yes)"), export_executable_symbols: bool = (false, parse_bool, [TRACKED], "export symbols from executables, as if they were dynamic libraries"), + external_clangrt: bool = (false, parse_bool, [UNTRACKED], + "rely on user specified linker commands to find clangrt"), extra_const_ub_checks: bool = (false, parse_bool, [TRACKED], "turns on more checks to detect const UB, which can be slow (default: no)"), #[rustc_lint_opt_deny_field_access("use `Session::fewer_names` instead of this field")] From bf62d5913a702754d46a0e9210fcf608deba63af Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 9 Feb 2024 16:16:22 +1100 Subject: [PATCH 02/32] Give `TRACK_DIAGNOSTIC` a return value. This means `DiagCtxtInner::emit_diagnostic` can return its result directly, rather than having to modify a local variable. --- compiler/rustc_errors/src/lib.rs | 24 ++++++++++++----------- compiler/rustc_interface/src/callbacks.rs | 10 +++++----- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 0a533833e64bd..ce24f1ceaac7b 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -526,12 +526,15 @@ pub enum StashKey { UndeterminedMacroResolution, } -fn default_track_diagnostic(diag: DiagInner, f: &mut dyn FnMut(DiagInner)) { +fn default_track_diagnostic(diag: DiagInner, f: &mut dyn FnMut(DiagInner) -> R) -> R { (*f)(diag) } -pub static TRACK_DIAGNOSTIC: AtomicRef = - AtomicRef::new(&(default_track_diagnostic as _)); +/// Diagnostics emitted by `DiagCtxtInner::emit_diagnostic` are passed through this function. Used +/// for tracking by incremental, to replay diagnostics as necessary. +pub static TRACK_DIAGNOSTIC: AtomicRef< + fn(DiagInner, &mut dyn FnMut(DiagInner) -> Option) -> Option, +> = AtomicRef::new(&(default_track_diagnostic as _)); #[derive(Copy, Clone, Default)] pub struct DiagCtxtFlags { @@ -1406,19 +1409,18 @@ impl DiagCtxtInner { } Warning if !self.flags.can_emit_warnings => { if diagnostic.has_future_breakage() { - (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); + TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); } return None; } Allow | Expect(_) => { - (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); + TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); return None; } _ => {} } - let mut guaranteed = None; - (*TRACK_DIAGNOSTIC)(diagnostic, &mut |mut diagnostic| { + TRACK_DIAGNOSTIC(diagnostic, &mut |mut diagnostic| { if let Some(code) = diagnostic.code { self.emitted_diagnostic_codes.insert(code); } @@ -1481,17 +1483,17 @@ impl DiagCtxtInner { // `ErrorGuaranteed` for errors and lint errors originates. #[allow(deprecated)] let guar = ErrorGuaranteed::unchecked_error_guaranteed(); - guaranteed = Some(guar); if is_lint { self.lint_err_guars.push(guar); } else { self.err_guars.push(guar); } self.panic_if_treat_err_as_bug(); + Some(guar) + } else { + None } - }); - - guaranteed + }) } fn treat_err_as_bug(&self) -> bool { diff --git a/compiler/rustc_interface/src/callbacks.rs b/compiler/rustc_interface/src/callbacks.rs index f44ae705a3c27..a27f73789cdef 100644 --- a/compiler/rustc_interface/src/callbacks.rs +++ b/compiler/rustc_interface/src/callbacks.rs @@ -29,7 +29,7 @@ fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) { /// This is a callback from `rustc_errors` as it cannot access the implicit state /// in `rustc_middle` otherwise. It is used when diagnostic messages are /// emitted and stores them in the current query, if there is one. -fn track_diagnostic(diagnostic: DiagInner, f: &mut dyn FnMut(DiagInner)) { +fn track_diagnostic(diagnostic: DiagInner, f: &mut dyn FnMut(DiagInner) -> R) -> R { tls::with_context_opt(|icx| { if let Some(icx) = icx { if let Some(diagnostics) = icx.diagnostics { @@ -38,11 +38,11 @@ fn track_diagnostic(diagnostic: DiagInner, f: &mut dyn FnMut(DiagInner)) { // Diagnostics are tracked, we can ignore the dependency. let icx = tls::ImplicitCtxt { task_deps: TaskDepsRef::Ignore, ..icx.clone() }; - return tls::enter_context(&icx, move || (*f)(diagnostic)); + tls::enter_context(&icx, move || (*f)(diagnostic)) + } else { + // In any other case, invoke diagnostics anyway. + (*f)(diagnostic) } - - // In any other case, invoke diagnostics anyway. - (*f)(diagnostic); }) } From ecd3718bc0b3f823b96d6949ba22f77cfbeff5c1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 12:20:38 +1100 Subject: [PATCH 03/32] Inline and remove `Level::get_diagnostic_id`. It has a single call site, and this will enable subsequent refactorings. --- compiler/rustc_errors/src/lib.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index ce24f1ceaac7b..0c8308b6b09b6 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1356,17 +1356,17 @@ impl DiagCtxtInner { fn emit_diagnostic(&mut self, mut diagnostic: DiagInner) -> Option { assert!(diagnostic.level.can_be_top_or_sub().0); - if let Some(expectation_id) = diagnostic.level.get_expectation_id() { + if let Expect(expect_id) | ForceWarning(Some(expect_id)) = diagnostic.level { // The `LintExpectationId` can be stable or unstable depending on when it was created. // Diagnostics created before the definition of `HirId`s are unstable and can not yet // be stored. Instead, they are buffered until the `LintExpectationId` is replaced by // a stable one by the `LintLevelsBuilder`. - if let LintExpectationId::Unstable { .. } = expectation_id { + if let LintExpectationId::Unstable { .. } = expect_id { self.unstable_expect_diagnostics.push(diagnostic); return None; } self.suppressed_expected_diag = true; - self.fulfilled_expectations.insert(expectation_id.normalize()); + self.fulfilled_expectations.insert(expect_id.normalize()); } if diagnostic.has_future_breakage() { @@ -1794,13 +1794,6 @@ impl Level { matches!(*self, FailureNote) } - pub fn get_expectation_id(&self) -> Option { - match self { - Expect(id) | ForceWarning(Some(id)) => Some(*id), - _ => None, - } - } - // Can this level be used in a top-level diagnostic message and/or a // subdiagnostic message? fn can_be_top_or_sub(&self) -> (bool, bool) { From 272e60bd3ef5ea2424b70b0f4ec821f38fa3dc79 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 13:08:24 +1100 Subject: [PATCH 04/32] Move `DelayedBug` handling into the `match`. It results in a tiny bit of duplication (another `self.treat_next_err_as_bug()` condition) but I think it's worth it to get more code into the main `match`. --- compiler/rustc_errors/src/lib.rs | 51 ++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 0c8308b6b09b6..2280d3a24172d 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1377,35 +1377,40 @@ impl DiagCtxtInner { self.future_breakage_diagnostics.push(diagnostic.clone()); } - // Note that because this comes before the `match` below, - // `-Zeagerly-emit-delayed-bugs` continues to work even after we've - // issued an error and stopped recording new delayed bugs. - if diagnostic.level == DelayedBug && self.flags.eagerly_emit_delayed_bugs { - diagnostic.level = Error; - } - match diagnostic.level { - // This must come after the possible promotion of `DelayedBug` to - // `Error` above. Fatal | Error if self.treat_next_err_as_bug() => { + // `Fatal` and `Error` can be promoted to `Bug`. diagnostic.level = Bug; } DelayedBug => { - // If we have already emitted at least one error, we don't need - // to record the delayed bug, because it'll never be used. - return if let Some(guar) = self.has_errors() { - Some(guar) + // Note that because we check these conditions first, + // `-Zeagerly-emit-delayed-bugs` and `-Ztreat-err-as-bug` + // continue to work even after we've issued an error and + // stopped recording new delayed bugs. + if self.flags.eagerly_emit_delayed_bugs { + // `DelayedBug` can be promoted to `Error` or `Bug`. + if self.treat_next_err_as_bug() { + diagnostic.level = Bug; + } else { + diagnostic.level = Error; + } } else { - let backtrace = std::backtrace::Backtrace::capture(); - // This `unchecked_error_guaranteed` is valid. It is where the - // `ErrorGuaranteed` for delayed bugs originates. See - // `DiagCtxtInner::drop`. - #[allow(deprecated)] - let guar = ErrorGuaranteed::unchecked_error_guaranteed(); - self.delayed_bugs - .push((DelayedDiagInner::with_backtrace(diagnostic, backtrace), guar)); - Some(guar) - }; + // If we have already emitted at least one error, we don't need + // to record the delayed bug, because it'll never be used. + return if let Some(guar) = self.has_errors() { + Some(guar) + } else { + let backtrace = std::backtrace::Backtrace::capture(); + // This `unchecked_error_guaranteed` is valid. It is where the + // `ErrorGuaranteed` for delayed bugs originates. See + // `DiagCtxtInner::drop`. + #[allow(deprecated)] + let guar = ErrorGuaranteed::unchecked_error_guaranteed(); + self.delayed_bugs + .push((DelayedDiagInner::with_backtrace(diagnostic, backtrace), guar)); + Some(guar) + }; + } } Warning if !self.flags.can_emit_warnings => { if diagnostic.has_future_breakage() { From c81767e7cb06dacc7bb9a5ec0b746b0c774e0aa1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 13:14:41 +1100 Subject: [PATCH 05/32] Reorder `has_future_breakage` handling. This will enable additional refactorings. --- compiler/rustc_errors/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 2280d3a24172d..e073520870905 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1356,6 +1356,14 @@ impl DiagCtxtInner { fn emit_diagnostic(&mut self, mut diagnostic: DiagInner) -> Option { assert!(diagnostic.level.can_be_top_or_sub().0); + if diagnostic.has_future_breakage() { + // Future breakages aren't emitted if they're Level::Allow, + // but they still need to be constructed and stashed below, + // so they'll trigger the must_produce_diag check. + self.suppressed_expected_diag = true; + self.future_breakage_diagnostics.push(diagnostic.clone()); + } + if let Expect(expect_id) | ForceWarning(Some(expect_id)) = diagnostic.level { // The `LintExpectationId` can be stable or unstable depending on when it was created. // Diagnostics created before the definition of `HirId`s are unstable and can not yet @@ -1369,14 +1377,6 @@ impl DiagCtxtInner { self.fulfilled_expectations.insert(expect_id.normalize()); } - if diagnostic.has_future_breakage() { - // Future breakages aren't emitted if they're Level::Allow, - // but they still need to be constructed and stashed below, - // so they'll trigger the must_produce_diag check. - self.suppressed_expected_diag = true; - self.future_breakage_diagnostics.push(diagnostic.clone()); - } - match diagnostic.level { Fatal | Error if self.treat_next_err_as_bug() => { // `Fatal` and `Error` can be promoted to `Bug`. From aec4bdb0a2802765ee90e4f59c9173f94f28f2eb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 14:20:41 +1100 Subject: [PATCH 06/32] Move `Expect`/`ForceWarning` handling into the `match`. Note that `self.suppressed_expected_diag` is no longer set for `ForceWarning`, which is good. Nor is `TRACK_DIAGNOSTIC` called for `Allow`, which is also good. --- compiler/rustc_errors/src/lib.rs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index e073520870905..0f3999062913f 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1364,19 +1364,6 @@ impl DiagCtxtInner { self.future_breakage_diagnostics.push(diagnostic.clone()); } - if let Expect(expect_id) | ForceWarning(Some(expect_id)) = diagnostic.level { - // The `LintExpectationId` can be stable or unstable depending on when it was created. - // Diagnostics created before the definition of `HirId`s are unstable and can not yet - // be stored. Instead, they are buffered until the `LintExpectationId` is replaced by - // a stable one by the `LintLevelsBuilder`. - if let LintExpectationId::Unstable { .. } = expect_id { - self.unstable_expect_diagnostics.push(diagnostic); - return None; - } - self.suppressed_expected_diag = true; - self.fulfilled_expectations.insert(expect_id.normalize()); - } - match diagnostic.level { Fatal | Error if self.treat_next_err_as_bug() => { // `Fatal` and `Error` can be promoted to `Bug`. @@ -1418,10 +1405,25 @@ impl DiagCtxtInner { } return None; } - Allow | Expect(_) => { - TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); + Allow => { return None; } + Expect(expect_id) | ForceWarning(Some(expect_id)) => { + // Diagnostics created before the definition of `HirId`s are + // unstable and can not yet be stored. Instead, they are + // buffered until the `LintExpectationId` is replaced by a + // stable one by the `LintLevelsBuilder`. + if let LintExpectationId::Unstable { .. } = expect_id { + self.unstable_expect_diagnostics.push(diagnostic); + return None; + } + self.fulfilled_expectations.insert(expect_id.normalize()); + if let Expect(_) = diagnostic.level { + TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); + self.suppressed_expected_diag = true; + return None; + } + } _ => {} } From a7d926265f25abfd05b13a3a0144f2ad9dd02dd7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Feb 2024 14:25:29 +1100 Subject: [PATCH 07/32] Add comments about `TRACK_DIAGNOSTIC` use. Also add an assertion for the levels allowed with `has_future_breakage`. --- compiler/rustc_errors/src/lib.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 0f3999062913f..738016f9b999c 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1360,10 +1360,13 @@ impl DiagCtxtInner { // Future breakages aren't emitted if they're Level::Allow, // but they still need to be constructed and stashed below, // so they'll trigger the must_produce_diag check. - self.suppressed_expected_diag = true; + assert!(matches!(diagnostic.level, Error | Warning | Allow)); self.future_breakage_diagnostics.push(diagnostic.clone()); } + // We call TRACK_DIAGNOSTIC with an empty closure for the cases that + // return early *and* have some kind of side-effect, except where + // noted. match diagnostic.level { Fatal | Error if self.treat_next_err_as_bug() => { // `Fatal` and `Error` can be promoted to `Bug`. @@ -1387,6 +1390,9 @@ impl DiagCtxtInner { return if let Some(guar) = self.has_errors() { Some(guar) } else { + // No `TRACK_DIAGNOSTIC` call is needed, because the + // incremental session is deleted if there is a delayed + // bug. This also saves us from cloning the diagnostic. let backtrace = std::backtrace::Backtrace::capture(); // This `unchecked_error_guaranteed` is valid. It is where the // `ErrorGuaranteed` for delayed bugs originates. See @@ -1401,11 +1407,17 @@ impl DiagCtxtInner { } Warning if !self.flags.can_emit_warnings => { if diagnostic.has_future_breakage() { + // The side-effect is at the top of this method. TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); } return None; } Allow => { + if diagnostic.has_future_breakage() { + // The side-effect is at the top of this method. + TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); + self.suppressed_expected_diag = true; + } return None; } Expect(expect_id) | ForceWarning(Some(expect_id)) => { @@ -1414,6 +1426,9 @@ impl DiagCtxtInner { // buffered until the `LintExpectationId` is replaced by a // stable one by the `LintLevelsBuilder`. if let LintExpectationId::Unstable { .. } = expect_id { + // We don't call TRACK_DIAGNOSTIC because we wait for the + // unstable ID to be updated, whereupon the diagnostic will + // be passed into this method again. self.unstable_expect_diagnostics.push(diagnostic); return None; } From 7ef605be3fa24f93194a3faf4f75d3a05ceca78a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 1 Mar 2024 13:46:00 +1100 Subject: [PATCH 08/32] Make the `match` in `emit_diagnostic` complete. This match is complex enough that it's a good idea to enumerate every variant. This also means `can_be_top_or_sub` can just be `can_be_subdiag`. --- compiler/rustc_errors/src/emitter.rs | 2 +- compiler/rustc_errors/src/lib.rs | 41 ++++++++++++++++------------ 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 5637c05d04c69..212bda5ff0320 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -2104,7 +2104,7 @@ impl HumanEmitter { } if !self.short_message { for child in children { - assert!(child.level.can_be_top_or_sub().1); + assert!(child.level.can_be_subdiag()); let span = &child.span; if let Err(err) = self.emit_messages_default_inner( span, diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 738016f9b999c..069ac863d9939 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1354,8 +1354,6 @@ impl DiagCtxtInner { // Return value is only `Some` if the level is `Error` or `DelayedBug`. fn emit_diagnostic(&mut self, mut diagnostic: DiagInner) -> Option { - assert!(diagnostic.level.can_be_top_or_sub().0); - if diagnostic.has_future_breakage() { // Future breakages aren't emitted if they're Level::Allow, // but they still need to be constructed and stashed below, @@ -1368,9 +1366,12 @@ impl DiagCtxtInner { // return early *and* have some kind of side-effect, except where // noted. match diagnostic.level { - Fatal | Error if self.treat_next_err_as_bug() => { - // `Fatal` and `Error` can be promoted to `Bug`. - diagnostic.level = Bug; + Bug => {} + Fatal | Error => { + if self.treat_next_err_as_bug() { + // `Fatal` and `Error` can be promoted to `Bug`. + diagnostic.level = Bug; + } } DelayedBug => { // Note that because we check these conditions first, @@ -1405,14 +1406,21 @@ impl DiagCtxtInner { }; } } - Warning if !self.flags.can_emit_warnings => { - if diagnostic.has_future_breakage() { - // The side-effect is at the top of this method. - TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); + ForceWarning(None) => {} // `ForceWarning(Some(...))` is below, with `Expect` + Warning => { + if !self.flags.can_emit_warnings { + // We are not emitting warnings. + if diagnostic.has_future_breakage() { + // The side-effect is at the top of this method. + TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); + } + return None; } - return None; } + Note | Help | FailureNote => {} + OnceNote | OnceHelp => panic!("bad level: {:?}", diagnostic.level), Allow => { + // Nothing emitted for allowed lints. if diagnostic.has_future_breakage() { // The side-effect is at the top of this method. TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); @@ -1434,12 +1442,12 @@ impl DiagCtxtInner { } self.fulfilled_expectations.insert(expect_id.normalize()); if let Expect(_) = diagnostic.level { + // Nothing emitted here for expected lints. TRACK_DIAGNOSTIC(diagnostic, &mut |_| None); self.suppressed_expected_diag = true; return None; } } - _ => {} } TRACK_DIAGNOSTIC(diagnostic, &mut |mut diagnostic| { @@ -1816,16 +1824,13 @@ impl Level { matches!(*self, FailureNote) } - // Can this level be used in a top-level diagnostic message and/or a - // subdiagnostic message? - fn can_be_top_or_sub(&self) -> (bool, bool) { + // Can this level be used in a subdiagnostic message? + fn can_be_subdiag(&self) -> bool { match self { Bug | DelayedBug | Fatal | Error | ForceWarning(_) | FailureNote | Allow - | Expect(_) => (true, false), - - Warning | Note | Help => (true, true), + | Expect(_) => false, - OnceNote | OnceHelp => (false, true), + Warning | Note | Help | OnceNote | OnceHelp => true, } } } From 15b71f491f39c0d2e3b733c13328f3b513900309 Mon Sep 17 00:00:00 2001 From: ltdk Date: Sun, 13 Nov 2022 04:09:20 -0500 Subject: [PATCH 09/32] Add CStr::bytes iterator --- library/core/src/ffi/c_str.rs | 89 +++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 111fb83088b82..1dafc8c1f868c 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -5,8 +5,11 @@ use crate::error::Error; use crate::ffi::c_char; use crate::fmt; use crate::intrinsics; +use crate::iter::FusedIterator; +use crate::marker::PhantomData; use crate::ops; use crate::ptr::addr_of; +use crate::ptr::NonNull; use crate::slice; use crate::slice::memchr; use crate::str; @@ -617,6 +620,26 @@ impl CStr { unsafe { &*(addr_of!(self.inner) as *const [u8]) } } + /// Iterates over the bytes in this C string. + /// + /// The returned iterator will **not** contain the trailing nul terminator + /// that this C string has. + /// + /// # Examples + /// + /// ``` + /// #![feature(cstr_bytes)] + /// use std::ffi::CStr; + /// + /// let cstr = CStr::from_bytes_with_nul(b"foo\0").expect("CStr::from_bytes_with_nul failed"); + /// assert!(cstr.bytes().eq(*b"foo")); + /// ``` + #[inline] + #[unstable(feature = "cstr_bytes", issue = "112115")] + pub fn bytes(&self) -> Bytes<'_> { + Bytes::new(self) + } + /// Yields a &[str] slice if the `CStr` contains valid UTF-8. /// /// If the contents of the `CStr` are valid UTF-8 data, this @@ -735,3 +758,69 @@ const unsafe fn const_strlen(ptr: *const c_char) -> usize { intrinsics::const_eval_select((ptr,), strlen_ct, strlen_rt) } } + +/// An iterator over the bytes of a [`CStr`], without the nul terminator. +/// +/// This struct is created by the [`bytes`] method on [`CStr`]. +/// See its documentation for more. +/// +/// [`bytes`]: CStr::bytes +#[must_use = "iterators are lazy and do nothing unless consumed"] +#[unstable(feature = "cstr_bytes", issue = "112115")] +#[derive(Clone, Debug)] +pub struct Bytes<'a> { + // since we know the string is nul-terminated, we only need one pointer + ptr: NonNull, + phantom: PhantomData<&'a u8>, +} +impl<'a> Bytes<'a> { + #[inline] + fn new(s: &'a CStr) -> Self { + Self { + // SAFETY: Because we have a valid reference to the string, we know + // that its pointer is non-null. + ptr: unsafe { NonNull::new_unchecked(s.as_ptr() as *const u8 as *mut u8) }, + phantom: PhantomData, + } + } + + #[inline] + fn is_empty(&self) -> bool { + // SAFETY: We uphold that the pointer is always valid to dereference + // by starting with a valid C string and then never incrementing beyond + // the nul terminator. + unsafe { *self.ptr.as_ref() == 0 } + } +} + +#[unstable(feature = "cstr_bytes", issue = "112115")] +impl Iterator for Bytes<'_> { + type Item = u8; + + #[inline] + fn next(&mut self) -> Option { + // SAFETY: We only choose a pointer from a valid C string, which must + // be non-null and contain at least one value. Since we always stop at + // the nul terminator, which is guaranteed to exist, we can assume that + // the pointer is non-null and valid. This lets us safely dereference + // it and assume that adding 1 will create a new, non-null, valid + // pointer. + unsafe { + let ret = *self.ptr.as_ref(); + if ret == 0 { + None + } else { + self.ptr = NonNull::new_unchecked(self.ptr.as_ptr().offset(1)); + Some(ret) + } + } + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + if self.is_empty() { (0, Some(0)) } else { (1, None) } + } +} + +#[unstable(feature = "cstr_bytes", issue = "112115")] +impl FusedIterator for Bytes<'_> {} From a38a556ceb4b65c397a47bcbe35d004bc9b8626f Mon Sep 17 00:00:00 2001 From: ltdk Date: Tue, 12 Mar 2024 17:19:58 -0400 Subject: [PATCH 10/32] Reduce unsafe code, use more NonNull APIs per @cuviper review --- library/core/src/ffi/c_str.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 1dafc8c1f868c..30debbffec1aa 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -507,6 +507,13 @@ impl CStr { self.inner.as_ptr() } + /// We could eventually expose this publicly, if we wanted. + #[inline] + #[must_use] + const fn as_non_null_ptr(&self) -> NonNull { + NonNull::from(&self.inner).as_non_null_ptr() + } + /// Returns the length of `self`. Like C's `strlen`, this does not include the nul terminator. /// /// > **Note**: This method is currently implemented as a constant-time @@ -776,12 +783,7 @@ pub struct Bytes<'a> { impl<'a> Bytes<'a> { #[inline] fn new(s: &'a CStr) -> Self { - Self { - // SAFETY: Because we have a valid reference to the string, we know - // that its pointer is non-null. - ptr: unsafe { NonNull::new_unchecked(s.as_ptr() as *const u8 as *mut u8) }, - phantom: PhantomData, - } + Self { ptr: s.as_non_null_ptr().cast(), phantom: PhantomData } } #[inline] @@ -789,7 +791,7 @@ impl<'a> Bytes<'a> { // SAFETY: We uphold that the pointer is always valid to dereference // by starting with a valid C string and then never incrementing beyond // the nul terminator. - unsafe { *self.ptr.as_ref() == 0 } + unsafe { self.ptr.read() == 0 } } } @@ -806,11 +808,11 @@ impl Iterator for Bytes<'_> { // it and assume that adding 1 will create a new, non-null, valid // pointer. unsafe { - let ret = *self.ptr.as_ref(); + let ret = self.ptr.read(); if ret == 0 { None } else { - self.ptr = NonNull::new_unchecked(self.ptr.as_ptr().offset(1)); + self.ptr = self.ptr.offset(1); Some(ret) } } From f2fcfe82af65dd7a2fbc20ca082e1a6794918d0e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 13 Mar 2024 11:53:36 +1100 Subject: [PATCH 11/32] Various style improvements to `rustc_lint::levels` - Replace some nested if-let with let-chains - Tweak a match pattern to allow shorthand struct syntax - Fuse an `is_empty` check with getting the last element - Merge some common code that emits `MalformedAttribute` and continues - Format `"{tool}::{name}"` in a way that's consistent with other match arms - Replace if-let-else-panic with let-else - Use early-exit to flatten a method body --- compiler/rustc_lint/src/levels.rs | 249 +++++++++++++++--------------- 1 file changed, 121 insertions(+), 128 deletions(-) diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index e89df1c9840c6..74b6c92dbfed7 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -103,11 +103,12 @@ impl LintLevelSets { mut idx: LintStackIndex, aux: Option<&FxIndexMap>, ) -> (Option, LintLevelSource) { - if let Some(specs) = aux { - if let Some(&(level, src)) = specs.get(&id) { - return (Some(level), src); - } + if let Some(specs) = aux + && let Some(&(level, src)) = specs.get(&id) + { + return (Some(level), src); } + loop { let LintSet { ref specs, parent } = self.list[idx]; if let Some(&(level, src)) = specs.get(&id) { @@ -177,7 +178,7 @@ fn shallow_lint_levels_on(tcx: TyCtxt<'_>, owner: hir::OwnerId) -> ShallowLintLe // There is only something to do if there are attributes at all. [] => {} // Most of the time, there is only one attribute. Avoid fetching HIR in that case. - [(local_id, _)] => levels.add_id(HirId { owner, local_id: *local_id }), + &[(local_id, _)] => levels.add_id(HirId { owner, local_id }), // Otherwise, we need to visit the attributes in source code order, so we fetch HIR and do // a standard visit. // FIXME(#102522) Just iterate on attrs once that iteration order matches HIR's. @@ -643,63 +644,61 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { // // This means that this only errors if we're truly lowering the lint // level from forbid. - if self.lint_added_lints && level != Level::Forbid { - if let Level::Forbid = old_level { - // Backwards compatibility check: - // - // We used to not consider `forbid(lint_group)` - // as preventing `allow(lint)` for some lint `lint` in - // `lint_group`. For now, issue a future-compatibility - // warning for this case. - let id_name = id.lint.name_lower(); - let fcw_warning = match old_src { - LintLevelSource::Default => false, - LintLevelSource::Node { name, .. } => self.store.is_lint_group(name), - LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol), - }; - debug!( - "fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}", - fcw_warning, - self.current_specs(), - old_src, - id_name - ); - let sub = match old_src { - LintLevelSource::Default => { - OverruledAttributeSub::DefaultSource { id: id.to_string() } - } - LintLevelSource::Node { span, reason, .. } => { - OverruledAttributeSub::NodeSource { span, reason } - } - LintLevelSource::CommandLine(_, _) => OverruledAttributeSub::CommandLineSource, - }; - if !fcw_warning { - self.sess.dcx().emit_err(OverruledAttribute { - span: src.span(), + if self.lint_added_lints && level != Level::Forbid && old_level == Level::Forbid { + // Backwards compatibility check: + // + // We used to not consider `forbid(lint_group)` + // as preventing `allow(lint)` for some lint `lint` in + // `lint_group`. For now, issue a future-compatibility + // warning for this case. + let id_name = id.lint.name_lower(); + let fcw_warning = match old_src { + LintLevelSource::Default => false, + LintLevelSource::Node { name, .. } => self.store.is_lint_group(name), + LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol), + }; + debug!( + "fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}", + fcw_warning, + self.current_specs(), + old_src, + id_name + ); + let sub = match old_src { + LintLevelSource::Default => { + OverruledAttributeSub::DefaultSource { id: id.to_string() } + } + LintLevelSource::Node { span, reason, .. } => { + OverruledAttributeSub::NodeSource { span, reason } + } + LintLevelSource::CommandLine(_, _) => OverruledAttributeSub::CommandLineSource, + }; + if !fcw_warning { + self.sess.dcx().emit_err(OverruledAttribute { + span: src.span(), + overruled: src.span(), + lint_level: level.as_str(), + lint_source: src.name(), + sub, + }); + } else { + self.emit_span_lint( + FORBIDDEN_LINT_GROUPS, + src.span().into(), + OverruledAttributeLint { overruled: src.span(), lint_level: level.as_str(), lint_source: src.name(), sub, - }); - } else { - self.emit_span_lint( - FORBIDDEN_LINT_GROUPS, - src.span().into(), - OverruledAttributeLint { - overruled: src.span(), - lint_level: level.as_str(), - lint_source: src.name(), - sub, - }, - ); - } + }, + ); + } - // Retain the forbid lint level, unless we are - // issuing a FCW. In the FCW case, we want to - // respect the new setting. - if !fcw_warning { - return; - } + // Retain the forbid lint level, unless we are + // issuing a FCW. In the FCW case, we want to + // respect the new setting. + if !fcw_warning { + return; } } @@ -770,15 +769,15 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { let Some(mut metas) = attr.meta_item_list() else { continue }; - if metas.is_empty() { + // Check whether `metas` is empty, and get its last element. + let Some(tail_li) = metas.last() else { // This emits the unused_attributes lint for `#[level()]` continue; - } + }; // Before processing the lint names, look for a reason (RFC 2383) // at the end. let mut reason = None; - let tail_li = &metas[metas.len() - 1]; if let Some(item) = tail_li.meta_item() { match item.kind { ast::MetaItemKind::Word => {} // actual lint names handled later @@ -834,21 +833,16 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { let meta_item = match li { ast::NestedMetaItem::MetaItem(meta_item) if meta_item.is_word() => meta_item, _ => { - if let Some(item) = li.meta_item() { - if let ast::MetaItemKind::NameValue(_) = item.kind { - if item.path == sym::reason { - sess.dcx().emit_err(MalformedAttribute { - span: sp, - sub: MalformedAttributeSub::ReasonMustComeLast(sp), - }); - continue; - } - } - } - sess.dcx().emit_err(MalformedAttribute { - span: sp, - sub: MalformedAttributeSub::BadAttributeArgument(sp), - }); + let sub = if let Some(item) = li.meta_item() + && let ast::MetaItemKind::NameValue(_) = item.kind + && item.path == sym::reason + { + MalformedAttributeSub::ReasonMustComeLast(sp) + } else { + MalformedAttributeSub::BadAttributeArgument(sp) + }; + + sess.dcx().emit_err(MalformedAttribute { span: sp, sub }); continue; } }; @@ -987,11 +981,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { } CheckLintNameResult::NoLint(suggestion) => { - let name = if let Some(tool_ident) = tool_ident { - format!("{}::{}", tool_ident.name, name) - } else { - name.to_string() - }; + let name = tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name); let suggestion = suggestion.map(|(replace, from_rustc)| { UnknownLintSuggestion::WithSpan { suggestion: sp, replace, from_rustc } }); @@ -1005,27 +995,24 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { if let CheckLintNameResult::Renamed(new_name) = lint_result { // Ignore any errors or warnings that happen because the new name is inaccurate // NOTE: `new_name` already includes the tool name, so we don't have to add it again. - if let CheckLintNameResult::Ok(ids) = + let CheckLintNameResult::Ok(ids) = self.store.check_lint_name(&new_name, None, self.registered_tools) - { - let src = LintLevelSource::Node { - name: Symbol::intern(&new_name), - span: sp, - reason, - }; - for &id in ids { - if self.check_gated_lint(id, attr.span, false) { - self.insert_spec(id, (level, src)); - } - } - if let Level::Expect(expect_id) = level { - self.provider.push_expectation( - expect_id, - LintExpectation::new(reason, sp, false, tool_name), - ); - } - } else { + else { panic!("renamed lint does not exist: {new_name}"); + }; + + let src = + LintLevelSource::Node { name: Symbol::intern(&new_name), span: sp, reason }; + for &id in ids { + if self.check_gated_lint(id, attr.span, false) { + self.insert_spec(id, (level, src)); + } + } + if let Level::Expect(expect_id) = level { + self.provider.push_expectation( + expect_id, + LintExpectation::new(reason, sp, false, tool_name), + ); } } } @@ -1058,38 +1045,44 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> { /// Returns `true` if the lint's feature is enabled. #[track_caller] fn check_gated_lint(&self, lint_id: LintId, span: Span, lint_from_cli: bool) -> bool { - if let Some(feature) = lint_id.lint.feature_gate { - if !self.features.active(feature) { - if self.lint_added_lints { - let lint = builtin::UNKNOWN_LINTS; - let (level, src) = self.lint_level(builtin::UNKNOWN_LINTS); - // FIXME: make this translatable - #[allow(rustc::diagnostic_outside_of_impl)] - #[allow(rustc::untranslatable_diagnostic)] - lint_level( - self.sess, + let feature = if let Some(feature) = lint_id.lint.feature_gate + && !self.features.active(feature) + { + // Lint is behind a feature that is not enabled; eventually return false. + feature + } else { + // Lint is ungated or its feature is enabled; exit early. + return true; + }; + + if self.lint_added_lints { + let lint = builtin::UNKNOWN_LINTS; + let (level, src) = self.lint_level(builtin::UNKNOWN_LINTS); + // FIXME: make this translatable + #[allow(rustc::diagnostic_outside_of_impl)] + #[allow(rustc::untranslatable_diagnostic)] + lint_level( + self.sess, + lint, + level, + src, + Some(span.into()), + fluent::lint_unknown_gated_lint, + |lint| { + lint.arg("name", lint_id.lint.name_lower()); + lint.note(fluent::lint_note); + rustc_session::parse::add_feature_diagnostics_for_issue( lint, - level, - src, - Some(span.into()), - fluent::lint_unknown_gated_lint, - |lint| { - lint.arg("name", lint_id.lint.name_lower()); - lint.note(fluent::lint_note); - rustc_session::parse::add_feature_diagnostics_for_issue( - lint, - &self.sess, - feature, - GateIssue::Language, - lint_from_cli, - ); - }, + &self.sess, + feature, + GateIssue::Language, + lint_from_cli, ); - } - return false; - } + }, + ); } - true + + false } /// Find the lint level for a lint. From 8f21da1df333bdef7024266f6181ea7891bd3bea Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Mon, 11 Mar 2024 19:26:44 +0100 Subject: [PATCH 12/32] compiletest: Allow `only-unix` in test headers The header `ignore-unix` is already allowed. Also extend tests. --- src/tools/compiletest/src/header.rs | 1 + src/tools/compiletest/src/header/tests.rs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 69658222ff3eb..a512599f723b7 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -852,6 +852,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "only-sparc64", "only-stable", "only-thumb", + "only-unix", "only-wasm32", "only-wasm32-bare", "only-windows", diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 433f3e8b5559c..cf300fbe74feb 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -285,6 +285,7 @@ fn ignore_target() { assert!(check_ignore(&config, "//@ ignore-x86_64-unknown-linux-gnu")); assert!(check_ignore(&config, "//@ ignore-x86_64")); assert!(check_ignore(&config, "//@ ignore-linux")); + assert!(check_ignore(&config, "//@ ignore-unix")); assert!(check_ignore(&config, "//@ ignore-gnu")); assert!(check_ignore(&config, "//@ ignore-64bit")); @@ -300,6 +301,7 @@ fn only_target() { assert!(check_ignore(&config, "//@ only-x86")); assert!(check_ignore(&config, "//@ only-linux")); + assert!(check_ignore(&config, "//@ only-unix")); assert!(check_ignore(&config, "//@ only-msvc")); assert!(check_ignore(&config, "//@ only-32bit")); From ee8efd705b72584f7dff96a6c833b209b89d0b1e Mon Sep 17 00:00:00 2001 From: guoguangwu Date: Wed, 13 Mar 2024 13:57:23 +0800 Subject: [PATCH 13/32] fix: typos Signed-off-by: guoguangwu --- compiler/rustc_target/src/spec/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index c264a1fbce95c..941d767b850dc 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -211,7 +211,7 @@ impl ToJson for LldFlavor { impl LinkerFlavor { /// At this point the target's reference linker flavor doesn't yet exist and we need to infer - /// it. The inference always succeds and gives some result, and we don't report any flavor + /// it. The inference always succeeds and gives some result, and we don't report any flavor /// incompatibility errors for json target specs. The CLI flavor is used as the main source /// of truth, other flags are used in case of ambiguities. fn from_cli_json(cli: LinkerFlavorCli, lld_flavor: LldFlavor, is_gnu: bool) -> LinkerFlavor { @@ -581,7 +581,7 @@ impl LinkSelfContainedDefault { self == LinkSelfContainedDefault::False } - /// Returns whether the target spec explictly requests self-contained linking, i.e. not via + /// Returns whether the target spec explicitly requests self-contained linking, i.e. not via /// inference. pub fn is_linker_enabled(self) -> bool { match self { @@ -2090,7 +2090,7 @@ pub struct TargetOptions { /// If `None`, then `CFG_DEFAULT_CODEGEN_BACKEND` environmental variable captured when /// compiling `rustc` will be used instead (or llvm if it is not set). /// - /// N.B. when *using* the compiler, backend can always be overriden with `-Zcodegen-backend`. + /// N.B. when *using* the compiler, backend can always be overridden with `-Zcodegen-backend`. pub default_codegen_backend: Option>, /// Whether to generate trap instructions in places where optimization would From cdeeed8dec647fc6063416f5194336b687d93dec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 13 Mar 2024 08:31:07 +0100 Subject: [PATCH 14/32] Increase timeout for new bors bot --- rust-bors.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-bors.toml b/rust-bors.toml index 54f4f641248cd..f27eb2393675b 100644 --- a/rust-bors.toml +++ b/rust-bors.toml @@ -1 +1 @@ -timeout = 7200 +timeout = 14400 From e0488c0961e5ba693d19f6e1e3f4c9b20631353b Mon Sep 17 00:00:00 2001 From: "Celina G. Val" Date: Wed, 13 Mar 2024 00:36:54 -0700 Subject: [PATCH 15/32] Fix StableMIR is_full computation `WrappingRange::is_full` computation assumed that to be full the range couldn't wrap, which is not necessarily true. For example, a range of 1..=0 is a valid representation of a full wrapping range. --- compiler/stable_mir/src/abi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/stable_mir/src/abi.rs b/compiler/stable_mir/src/abi.rs index 7fda9ceb79ac2..92bc2e34561ae 100644 --- a/compiler/stable_mir/src/abi.rs +++ b/compiler/stable_mir/src/abi.rs @@ -383,7 +383,7 @@ impl WrappingRange { return Err(error!("Expected size <= 128 bits, but found {} instead", size.bits())); }; if self.start <= max_value && self.end <= max_value { - Ok(self.start == 0 && max_value == self.end) + Ok(self.start == (self.end.wrapping_add(1) & max_value)) } else { Err(error!("Range `{self:?}` out of bounds for size `{}` bits.", size.bits())) } From 90acda134c0852411629a763e400615ae9f038b0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 13 Mar 2024 09:20:16 +0000 Subject: [PATCH 16/32] Fix accidental re-addition of removed code in a previous PR --- compiler/rustc_const_eval/src/const_eval/eval_queries.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 5f4408ebbc6c2..a66a95a5f0c21 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -381,10 +381,7 @@ pub fn eval_in_interpreter<'mir, 'tcx>( Ok(mplace) => { // Since evaluation had no errors, validate the resulting constant. - // Temporarily allow access to the static_root_ids for the purpose of validation. - let static_root_ids = ecx.machine.static_root_ids.take(); let res = const_validate_mplace(&ecx, &mplace, cid); - ecx.machine.static_root_ids = static_root_ids; let alloc_id = mplace.ptr().provenance.unwrap().alloc_id(); From f10ebfe9fb5d994ec241eee915a1482f4d7570f9 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 11 Mar 2024 12:17:30 +0000 Subject: [PATCH 17/32] Generalize `eval_in_interpreter` with a helper trait --- .../src/const_eval/eval_queries.rs | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index a66a95a5f0c21..1b401cc5cc0d1 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -290,10 +290,36 @@ pub fn eval_static_initializer_provider<'tcx>( // they do not have to behave "as if" they were evaluated at runtime. CompileTimeInterpreter::new(CanAccessMutGlobal::Yes, CheckAlignment::Error), ); - let alloc_id = eval_in_interpreter(&mut ecx, cid, true)?.alloc_id; - let alloc = take_static_root_alloc(&mut ecx, alloc_id); - let alloc = tcx.mk_const_alloc(alloc); - Ok(alloc) + eval_in_interpreter(&mut ecx, cid, true) +} + +trait InterpretationResult<'tcx> { + /// This function takes the place where the result of the evaluation is stored + /// and prepares it for returning it in the appropriate format needed by the specific + /// evaluation query. + fn make_result<'mir>( + mplace: MPlaceTy<'tcx>, + ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + ) -> Self; +} + +impl<'tcx> InterpretationResult<'tcx> for mir::interpret::ConstAllocation<'tcx> { + fn make_result<'mir>( + mplace: MPlaceTy<'tcx>, + ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + ) -> Self { + let alloc = take_static_root_alloc(ecx, mplace.ptr().provenance.unwrap().alloc_id()); + ecx.tcx.mk_const_alloc(alloc) + } +} + +impl<'tcx> InterpretationResult<'tcx> for ConstAlloc<'tcx> { + fn make_result<'mir>( + mplace: MPlaceTy<'tcx>, + _ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + ) -> Self { + ConstAlloc { alloc_id: mplace.ptr().provenance.unwrap().alloc_id(), ty: mplace.layout.ty } + } } #[instrument(skip(tcx), level = "debug")] @@ -336,11 +362,11 @@ pub fn eval_to_allocation_raw_provider<'tcx>( eval_in_interpreter(&mut ecx, cid, is_static) } -pub fn eval_in_interpreter<'mir, 'tcx>( +fn eval_in_interpreter<'mir, 'tcx, R: InterpretationResult<'tcx>>( ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, cid: GlobalId<'tcx>, is_static: bool, -) -> ::rustc_middle::mir::interpret::EvalToAllocationRawResult<'tcx> { +) -> Result { // `is_static` just means "in static", it could still be a promoted! debug_assert_eq!(is_static, ecx.tcx.static_mutability(cid.instance.def_id()).is_some()); @@ -383,14 +409,12 @@ pub fn eval_in_interpreter<'mir, 'tcx>( let res = const_validate_mplace(&ecx, &mplace, cid); - let alloc_id = mplace.ptr().provenance.unwrap().alloc_id(); - // Validation failed, report an error. if let Err(error) = res { + let alloc_id = mplace.ptr().provenance.unwrap().alloc_id(); Err(const_report_error(&ecx, error, alloc_id)) } else { - // Convert to raw constant - Ok(ConstAlloc { alloc_id, ty: mplace.layout.ty }) + Ok(R::make_result(mplace, ecx)) } } } From 44b1f8a5c54118d6798f251da0fd57e9537a7829 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 11 Mar 2024 12:33:09 +0000 Subject: [PATCH 18/32] Move only usage of `take_static_root_alloc` to its definition and inline it --- .../src/const_eval/eval_queries.rs | 18 ++++------------ .../rustc_const_eval/src/interpret/intern.rs | 2 +- .../rustc_const_eval/src/interpret/mod.rs | 2 +- .../rustc_const_eval/src/interpret/util.rs | 21 ++++++++++++------- 4 files changed, 19 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 1b401cc5cc0d1..63b1d485a24f2 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -18,9 +18,9 @@ use crate::errors; use crate::errors::ConstEvalError; use crate::interpret::eval_nullary_intrinsic; use crate::interpret::{ - create_static_alloc, intern_const_alloc_recursive, take_static_root_alloc, CtfeValidationMode, - GlobalId, Immediate, InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, - OpTy, RefTracking, StackPopCleanup, + create_static_alloc, intern_const_alloc_recursive, CtfeValidationMode, GlobalId, Immediate, + InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, + StackPopCleanup, }; // Returns a pointer to where the result lives @@ -293,7 +293,7 @@ pub fn eval_static_initializer_provider<'tcx>( eval_in_interpreter(&mut ecx, cid, true) } -trait InterpretationResult<'tcx> { +pub trait InterpretationResult<'tcx> { /// This function takes the place where the result of the evaluation is stored /// and prepares it for returning it in the appropriate format needed by the specific /// evaluation query. @@ -303,16 +303,6 @@ trait InterpretationResult<'tcx> { ) -> Self; } -impl<'tcx> InterpretationResult<'tcx> for mir::interpret::ConstAllocation<'tcx> { - fn make_result<'mir>( - mplace: MPlaceTy<'tcx>, - ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, - ) -> Self { - let alloc = take_static_root_alloc(ecx, mplace.ptr().provenance.unwrap().alloc_id()); - ecx.tcx.mk_const_alloc(alloc) - } -} - impl<'tcx> InterpretationResult<'tcx> for ConstAlloc<'tcx> { fn make_result<'mir>( mplace: MPlaceTy<'tcx>, diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index 55d4b9edd59b8..b0fc0d7635065 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -175,7 +175,7 @@ pub fn intern_const_alloc_recursive< // This gives us the initial set of nested allocations, which will then all be processed // recursively in the loop below. let mut todo: Vec<_> = if is_static { - // Do not steal the root allocation, we need it later for `take_static_root_alloc` + // Do not steal the root allocation, we need it later to create the return value of `eval_static_initializer`. // But still change its mutability to match the requested one. let alloc = ecx.memory.alloc_map.get_mut(&base_alloc_id).unwrap(); alloc.1.mutability = base_mutability; diff --git a/compiler/rustc_const_eval/src/interpret/mod.rs b/compiler/rustc_const_eval/src/interpret/mod.rs index 2ed879ca72b5f..474d35b2aa3a2 100644 --- a/compiler/rustc_const_eval/src/interpret/mod.rs +++ b/compiler/rustc_const_eval/src/interpret/mod.rs @@ -39,5 +39,5 @@ use self::{ }; pub(crate) use self::intrinsics::eval_nullary_intrinsic; -pub(crate) use self::util::{create_static_alloc, take_static_root_alloc}; +pub(crate) use self::util::create_static_alloc; use eval_context::{from_known_layout, mir_assign_valid_types}; diff --git a/compiler/rustc_const_eval/src/interpret/util.rs b/compiler/rustc_const_eval/src/interpret/util.rs index 086475f72c5d4..c83ef14c03fe7 100644 --- a/compiler/rustc_const_eval/src/interpret/util.rs +++ b/compiler/rustc_const_eval/src/interpret/util.rs @@ -1,14 +1,15 @@ -use crate::const_eval::CompileTimeEvalContext; +use crate::const_eval::{CompileTimeEvalContext, CompileTimeInterpreter, InterpretationResult}; use crate::interpret::{MemPlaceMeta, MemoryKind}; use rustc_hir::def_id::LocalDefId; -use rustc_middle::mir::interpret::{AllocId, Allocation, InterpResult, Pointer}; +use rustc_middle::mir; +use rustc_middle::mir::interpret::{Allocation, InterpResult, Pointer}; use rustc_middle::ty::layout::TyAndLayout; use rustc_middle::ty::{ self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, }; use std::ops::ControlFlow; -use super::MPlaceTy; +use super::{InterpCx, MPlaceTy}; /// Checks whether a type contains generic parameters which must be instantiated. /// @@ -80,11 +81,15 @@ where } } -pub(crate) fn take_static_root_alloc<'mir, 'tcx: 'mir>( - ecx: &mut CompileTimeEvalContext<'mir, 'tcx>, - alloc_id: AllocId, -) -> Allocation { - ecx.memory.alloc_map.swap_remove(&alloc_id).unwrap().1 +impl<'tcx> InterpretationResult<'tcx> for mir::interpret::ConstAllocation<'tcx> { + fn make_result<'mir>( + mplace: MPlaceTy<'tcx>, + ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + ) -> Self { + let alloc_id = mplace.ptr().provenance.unwrap().alloc_id(); + let alloc = ecx.memory.alloc_map.swap_remove(&alloc_id).unwrap().1; + ecx.tcx.mk_const_alloc(alloc) + } } pub(crate) fn create_static_alloc<'mir, 'tcx: 'mir>( From 31fa1423770c12343d975f4459bad2c019631d05 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 11 Mar 2024 13:02:33 +0000 Subject: [PATCH 19/32] Move error handling into const_validate_mplace --- .../src/const_eval/eval_queries.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 63b1d485a24f2..6da8cf433c618 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -396,16 +396,9 @@ fn eval_in_interpreter<'mir, 'tcx, R: InterpretationResult<'tcx>>( } Ok(mplace) => { // Since evaluation had no errors, validate the resulting constant. + const_validate_mplace(&ecx, &mplace, cid)?; - let res = const_validate_mplace(&ecx, &mplace, cid); - - // Validation failed, report an error. - if let Err(error) = res { - let alloc_id = mplace.ptr().provenance.unwrap().alloc_id(); - Err(const_report_error(&ecx, error, alloc_id)) - } else { - Ok(R::make_result(mplace, ecx)) - } + Ok(R::make_result(mplace, ecx)) } } } @@ -415,7 +408,8 @@ pub fn const_validate_mplace<'mir, 'tcx>( ecx: &InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, mplace: &MPlaceTy<'tcx>, cid: GlobalId<'tcx>, -) -> InterpResult<'tcx> { +) -> Result<(), ErrorHandled> { + let alloc_id = mplace.ptr().provenance.unwrap().alloc_id(); let mut ref_tracking = RefTracking::new(mplace.clone()); let mut inner = false; while let Some((mplace, path)) = ref_tracking.todo.pop() { @@ -429,7 +423,8 @@ pub fn const_validate_mplace<'mir, 'tcx>( CtfeValidationMode::Const { allow_immutable_unsafe_cell: !inner } } }; - ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)?; + ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode) + .map_err(|error| const_report_error(&ecx, error, alloc_id))?; inner = true; } From 71ef9e29082d1cbb584d03f09a14689de1244a6b Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 11 Mar 2024 13:04:05 +0000 Subject: [PATCH 20/32] Move InterpCx into eval_in_interpreter --- .../src/const_eval/eval_queries.rs | 16 ++++++++-------- compiler/rustc_const_eval/src/interpret/util.rs | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 6da8cf433c618..3ad53731e8a79 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -282,7 +282,7 @@ pub fn eval_static_initializer_provider<'tcx>( let instance = ty::Instance::mono(tcx, def_id.to_def_id()); let cid = rustc_middle::mir::interpret::GlobalId { instance, promoted: None }; - let mut ecx = InterpCx::new( + let ecx = InterpCx::new( tcx, tcx.def_span(def_id), ty::ParamEnv::reveal_all(), @@ -290,7 +290,7 @@ pub fn eval_static_initializer_provider<'tcx>( // they do not have to behave "as if" they were evaluated at runtime. CompileTimeInterpreter::new(CanAccessMutGlobal::Yes, CheckAlignment::Error), ); - eval_in_interpreter(&mut ecx, cid, true) + eval_in_interpreter(ecx, cid, true) } pub trait InterpretationResult<'tcx> { @@ -299,14 +299,14 @@ pub trait InterpretationResult<'tcx> { /// evaluation query. fn make_result<'mir>( mplace: MPlaceTy<'tcx>, - ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, ) -> Self; } impl<'tcx> InterpretationResult<'tcx> for ConstAlloc<'tcx> { fn make_result<'mir>( mplace: MPlaceTy<'tcx>, - _ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + _ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, ) -> Self { ConstAlloc { alloc_id: mplace.ptr().provenance.unwrap().alloc_id(), ty: mplace.layout.ty } } @@ -339,7 +339,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>( let def = cid.instance.def.def_id(); let is_static = tcx.is_static(def); - let mut ecx = InterpCx::new( + let ecx = InterpCx::new( tcx, tcx.def_span(def), key.param_env, @@ -349,11 +349,11 @@ pub fn eval_to_allocation_raw_provider<'tcx>( // so we have to reject reading mutable global memory. CompileTimeInterpreter::new(CanAccessMutGlobal::from(is_static), CheckAlignment::Error), ); - eval_in_interpreter(&mut ecx, cid, is_static) + eval_in_interpreter(ecx, cid, is_static) } fn eval_in_interpreter<'mir, 'tcx, R: InterpretationResult<'tcx>>( - ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + mut ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, cid: GlobalId<'tcx>, is_static: bool, ) -> Result { @@ -361,7 +361,7 @@ fn eval_in_interpreter<'mir, 'tcx, R: InterpretationResult<'tcx>>( debug_assert_eq!(is_static, ecx.tcx.static_mutability(cid.instance.def_id()).is_some()); let res = ecx.load_mir(cid.instance.def, cid.promoted); - match res.and_then(|body| eval_body_using_ecx(ecx, cid, body)) { + match res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, body)) { Err(error) => { let (error, backtrace) = error.into_parts(); backtrace.print_backtrace(); diff --git a/compiler/rustc_const_eval/src/interpret/util.rs b/compiler/rustc_const_eval/src/interpret/util.rs index c83ef14c03fe7..10b5e3ff1df5a 100644 --- a/compiler/rustc_const_eval/src/interpret/util.rs +++ b/compiler/rustc_const_eval/src/interpret/util.rs @@ -84,7 +84,7 @@ where impl<'tcx> InterpretationResult<'tcx> for mir::interpret::ConstAllocation<'tcx> { fn make_result<'mir>( mplace: MPlaceTy<'tcx>, - ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + mut ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, ) -> Self { let alloc_id = mplace.ptr().provenance.unwrap().alloc_id(); let alloc = ecx.memory.alloc_map.swap_remove(&alloc_id).unwrap().1; From bd7580b4dcef274c0abab4ce9e930bb343b1a024 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 11 Mar 2024 13:20:12 +0000 Subject: [PATCH 21/32] Move generate_stacktrace_from_stack away from InterpCx to avoid having to know the `Machine` type --- .../rustc_const_eval/src/const_eval/error.rs | 9 +-- .../rustc_const_eval/src/const_eval/mod.rs | 2 +- .../src/interpret/eval_context.rs | 56 +++++++++---------- src/tools/miri/src/diagnostics.rs | 2 +- 4 files changed, 32 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/error.rs b/compiler/rustc_const_eval/src/const_eval/error.rs index b6adee435ba3a..c74f39dd1635a 100644 --- a/compiler/rustc_const_eval/src/const_eval/error.rs +++ b/compiler/rustc_const_eval/src/const_eval/error.rs @@ -8,9 +8,9 @@ use rustc_middle::ty::TyCtxt; use rustc_middle::ty::{layout::LayoutError, ConstInt}; use rustc_span::{Span, Symbol, DUMMY_SP}; -use super::{CompileTimeInterpreter, InterpCx}; +use super::CompileTimeInterpreter; use crate::errors::{self, FrameNote, ReportErrorExt}; -use crate::interpret::{ErrorHandled, InterpError, InterpErrorInfo, MachineStopType}; +use crate::interpret::{ErrorHandled, Frame, InterpError, InterpErrorInfo, MachineStopType}; /// The CTFE machine has some custom error kinds. #[derive(Clone, Debug)] @@ -63,10 +63,7 @@ pub fn get_span_and_frames<'tcx, 'mir>( where 'tcx: 'mir, { - let mut stacktrace = - InterpCx::>::generate_stacktrace_from_stack( - &machine.stack, - ); + let mut stacktrace = Frame::generate_stacktrace_from_stack(&machine.stack); // Filter out `requires_caller_location` frames. stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(*tcx)); let span = stacktrace.first().map(|f| f.span).unwrap_or(tcx.span); diff --git a/compiler/rustc_const_eval/src/const_eval/mod.rs b/compiler/rustc_const_eval/src/const_eval/mod.rs index 289dcb7d01d68..d0d6adbfad069 100644 --- a/compiler/rustc_const_eval/src/const_eval/mod.rs +++ b/compiler/rustc_const_eval/src/const_eval/mod.rs @@ -5,7 +5,7 @@ use rustc_middle::mir::interpret::InterpErrorInfo; use rustc_middle::query::TyCtxtAt; use rustc_middle::ty::{self, Ty}; -use crate::interpret::{format_interp_error, InterpCx}; +use crate::interpret::format_interp_error; mod error; mod eval_queries; diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index a484fbd892c6d..fbf9c141d22e7 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -279,6 +279,32 @@ impl<'mir, 'tcx, Prov: Provenance, Extra> Frame<'mir, 'tcx, Prov, Extra> { } }) } + + #[must_use] + pub fn generate_stacktrace_from_stack(stack: &[Self]) -> Vec> { + let mut frames = Vec::new(); + // This deliberately does *not* honor `requires_caller_location` since it is used for much + // more than just panics. + for frame in stack.iter().rev() { + let span = match frame.loc { + Left(loc) => { + // If the stacktrace passes through MIR-inlined source scopes, add them. + let mir::SourceInfo { mut span, scope } = *frame.body.source_info(loc); + let mut scope_data = &frame.body.source_scopes[scope]; + while let Some((instance, call_span)) = scope_data.inlined { + frames.push(FrameInfo { span, instance }); + span = call_span; + scope_data = &frame.body.source_scopes[scope_data.parent_scope.unwrap()]; + } + span + } + Right(span) => span, + }; + frames.push(FrameInfo { span, instance: frame.instance }); + } + trace!("generate stacktrace: {:#?}", frames); + frames + } } // FIXME: only used by miri, should be removed once translatable. @@ -1166,37 +1192,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { PlacePrinter { ecx: self, place: *place.place() } } - #[must_use] - pub fn generate_stacktrace_from_stack( - stack: &[Frame<'mir, 'tcx, M::Provenance, M::FrameExtra>], - ) -> Vec> { - let mut frames = Vec::new(); - // This deliberately does *not* honor `requires_caller_location` since it is used for much - // more than just panics. - for frame in stack.iter().rev() { - let span = match frame.loc { - Left(loc) => { - // If the stacktrace passes through MIR-inlined source scopes, add them. - let mir::SourceInfo { mut span, scope } = *frame.body.source_info(loc); - let mut scope_data = &frame.body.source_scopes[scope]; - while let Some((instance, call_span)) = scope_data.inlined { - frames.push(FrameInfo { span, instance }); - span = call_span; - scope_data = &frame.body.source_scopes[scope_data.parent_scope.unwrap()]; - } - span - } - Right(span) => span, - }; - frames.push(FrameInfo { span, instance: frame.instance }); - } - trace!("generate stacktrace: {:#?}", frames); - frames - } - #[must_use] pub fn generate_stacktrace(&self) -> Vec> { - Self::generate_stacktrace_from_stack(self.stack()) + Frame::generate_stacktrace_from_stack(self.stack()) } } diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 4683965159d73..6e612ea34a70f 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -528,7 +528,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { use NonHaltingDiagnostic::*; let stacktrace = - MiriInterpCx::generate_stacktrace_from_stack(self.threads.active_thread_stack()); + Frame::generate_stacktrace_from_stack(self.threads.active_thread_stack()); let (stacktrace, _was_pruned) = prune_stacktrace(stacktrace, self); let (title, diag_level) = match &e { From 7aee6657f280a96b273f29050ed85a15c5eac162 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 11 Mar 2024 13:21:42 +0000 Subject: [PATCH 22/32] Directly pass in the stack instead of computing it from a machine --- compiler/rustc_const_eval/src/const_eval/error.rs | 7 ++++--- compiler/rustc_const_eval/src/const_eval/eval_queries.rs | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/error.rs b/compiler/rustc_const_eval/src/const_eval/error.rs index c74f39dd1635a..763344207c467 100644 --- a/compiler/rustc_const_eval/src/const_eval/error.rs +++ b/compiler/rustc_const_eval/src/const_eval/error.rs @@ -2,6 +2,7 @@ use std::mem; use rustc_errors::{DiagArgName, DiagArgValue, DiagMessage, Diagnostic, IntoDiagArg}; use rustc_hir::CRATE_HIR_ID; +use rustc_middle::mir::interpret::Provenance; use rustc_middle::mir::AssertKind; use rustc_middle::query::TyCtxtAt; use rustc_middle::ty::TyCtxt; @@ -58,12 +59,12 @@ impl<'tcx> Into> for ConstEvalErrKind { pub fn get_span_and_frames<'tcx, 'mir>( tcx: TyCtxtAt<'tcx>, - machine: &CompileTimeInterpreter<'mir, 'tcx>, + stack: &[Frame<'mir, 'tcx, impl Provenance, impl Sized>], ) -> (Span, Vec) where 'tcx: 'mir, { - let mut stacktrace = Frame::generate_stacktrace_from_stack(&machine.stack); + let mut stacktrace = Frame::generate_stacktrace_from_stack(stack); // Filter out `requires_caller_location` frames. stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(*tcx)); let span = stacktrace.first().map(|f| f.span).unwrap_or(tcx.span); @@ -167,7 +168,7 @@ pub(super) fn lint<'tcx, 'mir, L>( ) where L: for<'a> rustc_errors::LintDiagnostic<'a, ()>, { - let (span, frames) = get_span_and_frames(tcx, machine); + let (span, frames) = get_span_and_frames(tcx, &machine.stack); tcx.emit_node_span_lint( lint, diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 3ad53731e8a79..439cb5b03b943 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -385,7 +385,7 @@ fn eval_in_interpreter<'mir, 'tcx, R: InterpretationResult<'tcx>>( *ecx.tcx, error, None, - || super::get_span_and_frames(ecx.tcx, &ecx.machine), + || super::get_span_and_frames(ecx.tcx, ecx.stack()), |span, frames| ConstEvalError { span, error_kind: kind, @@ -450,7 +450,7 @@ pub fn const_report_error<'mir, 'tcx>( *ecx.tcx, error, None, - || crate::const_eval::get_span_and_frames(ecx.tcx, &ecx.machine), + || crate::const_eval::get_span_and_frames(ecx.tcx, ecx.stack()), move |span, frames| errors::UndefinedBehavior { span, ub_note, frames, raw_bytes }, ) } From ffaf082feee14fc68cad2ef75c5400d128b3612c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 12 Mar 2024 09:34:57 +0000 Subject: [PATCH 23/32] Remove an argument that can be computed cheaply --- .../rustc_const_eval/src/const_eval/eval_queries.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 439cb5b03b943..7d0ce9930d5c7 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -290,7 +290,7 @@ pub fn eval_static_initializer_provider<'tcx>( // they do not have to behave "as if" they were evaluated at runtime. CompileTimeInterpreter::new(CanAccessMutGlobal::Yes, CheckAlignment::Error), ); - eval_in_interpreter(ecx, cid, true) + eval_in_interpreter(ecx, cid) } pub trait InterpretationResult<'tcx> { @@ -349,24 +349,20 @@ pub fn eval_to_allocation_raw_provider<'tcx>( // so we have to reject reading mutable global memory. CompileTimeInterpreter::new(CanAccessMutGlobal::from(is_static), CheckAlignment::Error), ); - eval_in_interpreter(ecx, cid, is_static) + eval_in_interpreter(ecx, cid) } fn eval_in_interpreter<'mir, 'tcx, R: InterpretationResult<'tcx>>( mut ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, cid: GlobalId<'tcx>, - is_static: bool, ) -> Result { - // `is_static` just means "in static", it could still be a promoted! - debug_assert_eq!(is_static, ecx.tcx.static_mutability(cid.instance.def_id()).is_some()); - let res = ecx.load_mir(cid.instance.def, cid.promoted); match res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, body)) { Err(error) => { let (error, backtrace) = error.into_parts(); backtrace.print_backtrace(); - let (kind, instance) = if is_static { + let (kind, instance) = if ecx.tcx.is_static(cid.instance.def_id()) { ("static", String::new()) } else { // If the current item has generics, we'd like to enrich the message with the From 66a46bbcb9887d7cf3501e71848023e5470daed2 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 12 Mar 2024 09:41:49 +0000 Subject: [PATCH 24/32] Share the `InterpCx` creation between static and const evaluation --- .../src/const_eval/eval_queries.rs | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 7d0ce9930d5c7..2608107826fcd 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -282,15 +282,7 @@ pub fn eval_static_initializer_provider<'tcx>( let instance = ty::Instance::mono(tcx, def_id.to_def_id()); let cid = rustc_middle::mir::interpret::GlobalId { instance, promoted: None }; - let ecx = InterpCx::new( - tcx, - tcx.def_span(def_id), - ty::ParamEnv::reveal_all(), - // Statics (and promoteds inside statics) may access other statics, because unlike consts - // they do not have to behave "as if" they were evaluated at runtime. - CompileTimeInterpreter::new(CanAccessMutGlobal::Yes, CheckAlignment::Error), - ); - eval_in_interpreter(ecx, cid) + eval_in_interpreter(tcx, cid, ty::ParamEnv::reveal_all()) } pub trait InterpretationResult<'tcx> { @@ -335,27 +327,27 @@ pub fn eval_to_allocation_raw_provider<'tcx>( trace!("const eval: {:?} ({})", key, instance); } - let cid = key.value; + eval_in_interpreter(tcx, key.value, key.param_env) +} + +fn eval_in_interpreter<'tcx, R: InterpretationResult<'tcx>>( + tcx: TyCtxt<'tcx>, + cid: GlobalId<'tcx>, + param_env: ty::ParamEnv<'tcx>, +) -> Result { let def = cid.instance.def.def_id(); let is_static = tcx.is_static(def); - let ecx = InterpCx::new( + let mut ecx = InterpCx::new( tcx, tcx.def_span(def), - key.param_env, + param_env, // Statics (and promoteds inside statics) may access mutable global memory, because unlike consts // they do not have to behave "as if" they were evaluated at runtime. // For consts however we want to ensure they behave "as if" they were evaluated at runtime, // so we have to reject reading mutable global memory. CompileTimeInterpreter::new(CanAccessMutGlobal::from(is_static), CheckAlignment::Error), ); - eval_in_interpreter(ecx, cid) -} - -fn eval_in_interpreter<'mir, 'tcx, R: InterpretationResult<'tcx>>( - mut ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, - cid: GlobalId<'tcx>, -) -> Result { let res = ecx.load_mir(cid.instance.def, cid.promoted); match res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, body)) { Err(error) => { From af59eec2b2f30db5ad54811ac4a95f6d7d62da55 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 12 Mar 2024 10:04:36 +0000 Subject: [PATCH 25/32] Move validation into eval_body_using_ecx --- .../rustc_const_eval/src/const_eval/eval_queries.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 2608107826fcd..4f41c977f2e20 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -84,6 +84,9 @@ fn eval_body_using_ecx<'mir, 'tcx>( // Intern the result intern_const_alloc_recursive(ecx, intern_kind, &ret)?; + // Since evaluation had no errors, validate the resulting constant. + const_validate_mplace(&ecx, &ret, cid)?; + Ok(ret) } @@ -382,12 +385,7 @@ fn eval_in_interpreter<'tcx, R: InterpretationResult<'tcx>>( }, )) } - Ok(mplace) => { - // Since evaluation had no errors, validate the resulting constant. - const_validate_mplace(&ecx, &mplace, cid)?; - - Ok(R::make_result(mplace, ecx)) - } + Ok(mplace) => Ok(R::make_result(mplace, ecx)), } } From 2a1a6fabe8e945a2fac62da37af8a644ed524eb4 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 12 Mar 2024 13:41:41 +0000 Subject: [PATCH 26/32] Move the entire success path into `eval_body_using_ecx` --- .../src/const_eval/eval_queries.rs | 72 +++++++++---------- .../rustc_const_eval/src/interpret/util.rs | 2 +- 2 files changed, 33 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 4f41c977f2e20..d62ab39d0ecfc 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -24,12 +24,12 @@ use crate::interpret::{ }; // Returns a pointer to where the result lives -#[instrument(level = "trace", skip(ecx, body), ret)] -fn eval_body_using_ecx<'mir, 'tcx>( +#[instrument(level = "trace", skip(ecx, body))] +fn eval_body_using_ecx<'mir, 'tcx, R: InterpretationResult<'tcx>>( ecx: &mut CompileTimeEvalContext<'mir, 'tcx>, cid: GlobalId<'tcx>, body: &'mir mir::Body<'tcx>, -) -> InterpResult<'tcx, MPlaceTy<'tcx>> { +) -> InterpResult<'tcx, R> { trace!(?ecx.param_env); let tcx = *ecx.tcx; assert!( @@ -87,7 +87,7 @@ fn eval_body_using_ecx<'mir, 'tcx>( // Since evaluation had no errors, validate the resulting constant. const_validate_mplace(&ecx, &ret, cid)?; - Ok(ret) + Ok(R::make_result(ret, ecx)) } /// The `InterpCx` is only meant to be used to do field and index projections into constants for @@ -294,14 +294,14 @@ pub trait InterpretationResult<'tcx> { /// evaluation query. fn make_result<'mir>( mplace: MPlaceTy<'tcx>, - ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, ) -> Self; } impl<'tcx> InterpretationResult<'tcx> for ConstAlloc<'tcx> { fn make_result<'mir>( mplace: MPlaceTy<'tcx>, - _ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + _ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, ) -> Self { ConstAlloc { alloc_id: mplace.ptr().provenance.unwrap().alloc_id(), ty: mplace.layout.ty } } @@ -352,41 +352,33 @@ fn eval_in_interpreter<'tcx, R: InterpretationResult<'tcx>>( CompileTimeInterpreter::new(CanAccessMutGlobal::from(is_static), CheckAlignment::Error), ); let res = ecx.load_mir(cid.instance.def, cid.promoted); - match res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, body)) { - Err(error) => { - let (error, backtrace) = error.into_parts(); - backtrace.print_backtrace(); - - let (kind, instance) = if ecx.tcx.is_static(cid.instance.def_id()) { - ("static", String::new()) + res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, body)).map_err(|error| { + let (error, backtrace) = error.into_parts(); + backtrace.print_backtrace(); + + let (kind, instance) = if ecx.tcx.is_static(cid.instance.def_id()) { + ("static", String::new()) + } else { + // If the current item has generics, we'd like to enrich the message with the + // instance and its args: to show the actual compile-time values, in addition to + // the expression, leading to the const eval error. + let instance = &cid.instance; + if !instance.args.is_empty() { + let instance = with_no_trimmed_paths!(instance.to_string()); + ("const_with_path", instance) } else { - // If the current item has generics, we'd like to enrich the message with the - // instance and its args: to show the actual compile-time values, in addition to - // the expression, leading to the const eval error. - let instance = &cid.instance; - if !instance.args.is_empty() { - let instance = with_no_trimmed_paths!(instance.to_string()); - ("const_with_path", instance) - } else { - ("const", String::new()) - } - }; - - Err(super::report( - *ecx.tcx, - error, - None, - || super::get_span_and_frames(ecx.tcx, ecx.stack()), - |span, frames| ConstEvalError { - span, - error_kind: kind, - instance, - frame_notes: frames, - }, - )) - } - Ok(mplace) => Ok(R::make_result(mplace, ecx)), - } + ("const", String::new()) + } + }; + + super::report( + *ecx.tcx, + error, + None, + || super::get_span_and_frames(ecx.tcx, ecx.stack()), + |span, frames| ConstEvalError { span, error_kind: kind, instance, frame_notes: frames }, + ) + }) } #[inline(always)] diff --git a/compiler/rustc_const_eval/src/interpret/util.rs b/compiler/rustc_const_eval/src/interpret/util.rs index 10b5e3ff1df5a..c83ef14c03fe7 100644 --- a/compiler/rustc_const_eval/src/interpret/util.rs +++ b/compiler/rustc_const_eval/src/interpret/util.rs @@ -84,7 +84,7 @@ where impl<'tcx> InterpretationResult<'tcx> for mir::interpret::ConstAllocation<'tcx> { fn make_result<'mir>( mplace: MPlaceTy<'tcx>, - mut ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, ) -> Self { let alloc_id = mplace.ptr().provenance.unwrap().alloc_id(); let alloc = ecx.memory.alloc_map.swap_remove(&alloc_id).unwrap().1; From d919b04e59b0e357b509cb9c0127dce45f532996 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 13 Mar 2024 11:24:51 +0100 Subject: [PATCH 27/32] Generate link to `Local` in `hir::Let` documentation --- compiler/rustc_hir/src/hir.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 7d5b4c0f06de2..fd069eb09e25e 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1253,11 +1253,11 @@ pub struct Arm<'hir> { pub body: &'hir Expr<'hir>, } -/// Represents a `let [: ] = ` expression (not a Local), occurring in an `if-let` or -/// `let-else`, evaluating to a boolean. Typically the pattern is refutable. +/// Represents a `let [: ] = ` expression (not a [`Local`]), occurring in an `if-let` +/// or `let-else`, evaluating to a boolean. Typically the pattern is refutable. /// -/// In an if-let, imagine it as `if (let = ) { ... }`; in a let-else, it is part of the -/// desugaring to if-let. Only let-else supports the type annotation at present. +/// In an `if let`, imagine it as `if (let = ) { ... }`; in a let-else, it is part of +/// the desugaring to if-let. Only let-else supports the type annotation at present. #[derive(Debug, Clone, Copy, HashStable_Generic)] pub struct Let<'hir> { pub span: Span, From 339322735ebe9540102c6fdc96382610bee1e1ed Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 13 Mar 2024 09:25:20 +0000 Subject: [PATCH 28/32] Rename some things around validation error reporting to signal that it is in fact about validation failures --- compiler/rustc_const_eval/messages.ftl | 12 ++++++------ .../rustc_const_eval/src/const_eval/eval_queries.rs | 10 ++++++---- compiler/rustc_const_eval/src/errors.rs | 6 +++--- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_const_eval/messages.ftl b/compiler/rustc_const_eval/messages.ftl index f3af633b4e561..0046190d20cc7 100644 --- a/compiler/rustc_const_eval/messages.ftl +++ b/compiler/rustc_const_eval/messages.ftl @@ -374,12 +374,6 @@ const_eval_unallowed_op_in_const_context = const_eval_unavailable_target_features_for_fn = calling a function that requires unavailable target features: {$unavailable_feats} -const_eval_undefined_behavior = - it is undefined behavior to use this value - -const_eval_undefined_behavior_note = - The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. - const_eval_uninhabited_enum_variant_read = read discriminant of an uninhabited enum variant const_eval_uninhabited_enum_variant_written = @@ -434,6 +428,12 @@ const_eval_validation_expected_raw_ptr = expected a raw pointer const_eval_validation_expected_ref = expected a reference const_eval_validation_expected_str = expected a string +const_eval_validation_failure = + it is undefined behavior to use this value + +const_eval_validation_failure_note = + The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. + const_eval_validation_front_matter_invalid_value = constructing invalid value const_eval_validation_front_matter_invalid_value_with_path = constructing invalid value at {$path} diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index d62ab39d0ecfc..5a1c7cc4209ad 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -382,7 +382,7 @@ fn eval_in_interpreter<'tcx, R: InterpretationResult<'tcx>>( } #[inline(always)] -pub fn const_validate_mplace<'mir, 'tcx>( +fn const_validate_mplace<'mir, 'tcx>( ecx: &InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, mplace: &MPlaceTy<'tcx>, cid: GlobalId<'tcx>, @@ -402,7 +402,9 @@ pub fn const_validate_mplace<'mir, 'tcx>( } }; ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode) - .map_err(|error| const_report_error(&ecx, error, alloc_id))?; + // Instead of just reporting the `InterpError` via the usual machinery, we give a more targetted + // error about the validation failure. + .map_err(|error| report_validation_error(&ecx, error, alloc_id))?; inner = true; } @@ -410,7 +412,7 @@ pub fn const_validate_mplace<'mir, 'tcx>( } #[inline(always)] -pub fn const_report_error<'mir, 'tcx>( +fn report_validation_error<'mir, 'tcx>( ecx: &InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, error: InterpErrorInfo<'tcx>, alloc_id: AllocId, @@ -429,6 +431,6 @@ pub fn const_report_error<'mir, 'tcx>( error, None, || crate::const_eval::get_span_and_frames(ecx.tcx, ecx.stack()), - move |span, frames| errors::UndefinedBehavior { span, ub_note, frames, raw_bytes }, + move |span, frames| errors::ValidationFailure { span, ub_note, frames, raw_bytes }, ) } diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index 249c02b75f7d2..077b411c60744 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -409,11 +409,11 @@ pub struct NullaryIntrinsicError { } #[derive(Diagnostic)] -#[diag(const_eval_undefined_behavior, code = E0080)] -pub struct UndefinedBehavior { +#[diag(const_eval_validation_failure, code = E0080)] +pub struct ValidationFailure { #[primary_span] pub span: Span, - #[note(const_eval_undefined_behavior_note)] + #[note(const_eval_validation_failure_note)] pub ub_note: Option<()>, #[subdiagnostic] pub frames: Vec, From cb15bf6256495201c6b94ae5e51fe88be6100c00 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 13 Mar 2024 13:53:18 +0100 Subject: [PATCH 29/32] Rename `ValidityConstraint` -> `PlaceValidity` The old name came from a time where I wanted to reuse it for differentiating wildcards from bindings. I don't plan to do this anymore. --- compiler/rustc_pattern_analysis/src/lib.rs | 4 +-- .../rustc_pattern_analysis/src/usefulness.rs | 28 +++++++------------ 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index f632eaf7ea4f6..41ae60a8b7ff5 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -179,10 +179,10 @@ pub fn analyze_match<'p, 'tcx>( pattern_complexity_limit: Option, ) -> Result, ErrorGuaranteed> { use lints::lint_nonexhaustive_missing_variants; - use usefulness::{compute_match_usefulness, ValidityConstraint}; + use usefulness::{compute_match_usefulness, PlaceValidity}; let scrut_ty = tycx.reveal_opaque_ty(scrut_ty); - let scrut_validity = ValidityConstraint::from_bool(tycx.known_valid_scrutinee); + let scrut_validity = PlaceValidity::from_bool(tycx.known_valid_scrutinee); let report = compute_match_usefulness(tycx, arms, scrut_ty, scrut_validity, pattern_complexity_limit)?; diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 0834d08106f3b..495785066ce62 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -540,8 +540,8 @@ //! We track in the algorithm whether a given place is known to contain valid data. This is done //! first by inspecting the scrutinee syntactically (which gives us `cx.known_valid_scrutinee`), and //! then by tracking validity of each column of the matrix (which correspond to places) as we -//! recurse into subpatterns. That second part is done through [`ValidityConstraint`], most notably -//! [`ValidityConstraint::specialize`]. +//! recurse into subpatterns. That second part is done through [`PlaceValidity`], most notably +//! [`PlaceValidity::specialize`]. //! //! Having said all that, in practice we don't fully follow what's been presented in this section. //! Let's call "toplevel exception" the case where the match scrutinee itself has type `!` or @@ -718,7 +718,7 @@ use crate::constructor::{Constructor, ConstructorSet, IntRange}; use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat}; use crate::{Captures, MatchArm, PrivateUninhabitedField, TypeCx}; -use self::ValidityConstraint::*; +use self::PlaceValidity::*; #[cfg(feature = "rustc")] use rustc_data_structures::stack::ensure_sufficient_stack; @@ -780,18 +780,14 @@ impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> { } } -/// Serves two purposes: -/// - in a wildcard, tracks whether the wildcard matches only valid values (i.e. is a binding `_a`) -/// or also invalid values (i.e. is a true `_` pattern). -/// - in the matrix, track whether a given place (aka column) is known to contain a valid value or -/// not. +/// Track whether a given place (aka column) is known to contain a valid value or not. #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub enum ValidityConstraint { +pub enum PlaceValidity { ValidOnly, MaybeInvalid, } -impl ValidityConstraint { +impl PlaceValidity { pub fn from_bool(is_valid_only: bool) -> Self { if is_valid_only { ValidOnly } else { MaybeInvalid } } @@ -817,7 +813,7 @@ impl ValidityConstraint { } } -impl fmt::Display for ValidityConstraint { +impl fmt::Display for PlaceValidity { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let s = match self { ValidOnly => "✓", @@ -836,7 +832,7 @@ struct PlaceInfo { /// so that we don't observe its emptiness. private_uninhabited: bool, /// Whether the place is known to contain valid data. - validity: ValidityConstraint, + validity: PlaceValidity, /// Whether the place is the scrutinee itself or a subplace of it. is_scrutinee: bool, } @@ -1155,11 +1151,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { } /// Build a new matrix from an iterator of `MatchArm`s. - fn new( - arms: &[MatchArm<'p, Cx>], - scrut_ty: Cx::Ty, - scrut_validity: ValidityConstraint, - ) -> Self { + fn new(arms: &[MatchArm<'p, Cx>], scrut_ty: Cx::Ty, scrut_validity: PlaceValidity) -> Self { let place_info = PlaceInfo { ty: scrut_ty, private_uninhabited: false, @@ -1754,7 +1746,7 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>( tycx: &Cx, arms: &[MatchArm<'p, Cx>], scrut_ty: Cx::Ty, - scrut_validity: ValidityConstraint, + scrut_validity: PlaceValidity, complexity_limit: Option, ) -> Result, Cx::Error> { let mut cx = UsefulnessCtxt { From 4fc35c46ff70821a37fec5f7a4c36087704d9f23 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 13 Mar 2024 13:56:06 +0100 Subject: [PATCH 30/32] Rename `TypeCx` -> `PatCx` --- .../rustc_pattern_analysis/src/constructor.rs | 18 ++--- compiler/rustc_pattern_analysis/src/lib.rs | 8 +- compiler/rustc_pattern_analysis/src/pat.rs | 26 +++---- .../rustc_pattern_analysis/src/pat_column.rs | 6 +- compiler/rustc_pattern_analysis/src/rustc.rs | 4 +- .../rustc_pattern_analysis/src/usefulness.rs | 74 +++++++++---------- 6 files changed, 68 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 2d55785cd0692..1faecb7e1dd8f 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -40,7 +40,7 @@ //! - That have no non-trivial intersection with any of the constructors in the column (i.e. they're //! each either disjoint with or covered by any given column constructor). //! -//! We compute this in two steps: first [`TypeCx::ctors_for_ty`] determines the +//! We compute this in two steps: first [`PatCx::ctors_for_ty`] determines the //! set of all possible constructors for the type. Then [`ConstructorSet::split`] looks at the //! column of constructors and splits the set into groups accordingly. The precise invariants of //! [`ConstructorSet::split`] is described in [`SplitConstructorSet`]. @@ -136,7 +136,7 @@ //! the algorithm can't distinguish them from a nonempty constructor. The only known case where this //! could happen is the `[..]` pattern on `[!; N]` with `N > 0` so we must take care to not emit it. //! -//! This is all handled by [`TypeCx::ctors_for_ty`] and +//! This is all handled by [`PatCx::ctors_for_ty`] and //! [`ConstructorSet::split`]. The invariants of [`SplitConstructorSet`] are also of interest. //! //! @@ -162,7 +162,7 @@ use self::MaybeInfiniteInt::*; use self::SliceKind::*; use crate::index; -use crate::TypeCx; +use crate::PatCx; /// Whether we have seen a constructor in the column or not. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] @@ -651,7 +651,7 @@ impl OpaqueId { /// constructor. `Constructor::apply` reconstructs the pattern from a pair of `Constructor` and /// `Fields`. #[derive(Debug)] -pub enum Constructor { +pub enum Constructor { /// Tuples and structs. Struct, /// Enum variants. @@ -696,7 +696,7 @@ pub enum Constructor { PrivateUninhabited, } -impl Clone for Constructor { +impl Clone for Constructor { fn clone(&self) -> Self { match self { Constructor::Struct => Constructor::Struct, @@ -720,7 +720,7 @@ impl Clone for Constructor { } } -impl Constructor { +impl Constructor { pub(crate) fn is_non_exhaustive(&self) -> bool { matches!(self, NonExhaustive) } @@ -838,7 +838,7 @@ pub enum VariantVisibility { /// In terms of division of responsibility, [`ConstructorSet::split`] handles all of the /// `exhaustive_patterns` feature. #[derive(Debug)] -pub enum ConstructorSet { +pub enum ConstructorSet { /// The type is a tuple or struct. `empty` tracks whether the type is empty. Struct { empty: bool }, /// This type has the following list of constructors. If `variants` is empty and @@ -889,13 +889,13 @@ pub enum ConstructorSet { /// of the `ConstructorSet` for the type, yet if we forgot to include them in `present` we would be /// ignoring any row with `Opaque`s in the algorithm. Hence the importance of point 4. #[derive(Debug)] -pub struct SplitConstructorSet { +pub struct SplitConstructorSet { pub present: SmallVec<[Constructor; 1]>, pub missing: Vec>, pub missing_empty: Vec>, } -impl ConstructorSet { +impl ConstructorSet { /// This analyzes a column of constructors to 1/ determine which constructors of the type (if /// any) are missing; 2/ split constructors to handle non-trivial intersections e.g. on ranges /// or slices. This can get subtle; see [`SplitConstructorSet`] for details of this operation diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 41ae60a8b7ff5..ff085d2a422d3 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -84,7 +84,7 @@ pub struct PrivateUninhabitedField(pub bool); /// Context that provides type information about constructors. /// /// Most of the crate is parameterized on a type that implements this trait. -pub trait TypeCx: Sized + fmt::Debug { +pub trait PatCx: Sized + fmt::Debug { /// The type of a pattern. type Ty: Clone + fmt::Debug; /// Errors that can abort analysis. @@ -155,19 +155,19 @@ pub trait TypeCx: Sized + fmt::Debug { /// The arm of a match expression. #[derive(Debug)] -pub struct MatchArm<'p, Cx: TypeCx> { +pub struct MatchArm<'p, Cx: PatCx> { pub pat: &'p DeconstructedPat, pub has_guard: bool, pub arm_data: Cx::ArmData, } -impl<'p, Cx: TypeCx> Clone for MatchArm<'p, Cx> { +impl<'p, Cx: PatCx> Clone for MatchArm<'p, Cx> { fn clone(&self) -> Self { Self { pat: self.pat, has_guard: self.has_guard, arm_data: self.arm_data } } } -impl<'p, Cx: TypeCx> Copy for MatchArm<'p, Cx> {} +impl<'p, Cx: PatCx> Copy for MatchArm<'p, Cx> {} /// The entrypoint for this crate. Computes whether a match is exhaustive and which of its arms are /// useful, and runs some lints. diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index cefc1d8e3b3a1..e3667d44bc976 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -5,7 +5,7 @@ use std::fmt; use smallvec::{smallvec, SmallVec}; use crate::constructor::{Constructor, Slice, SliceKind}; -use crate::{PrivateUninhabitedField, TypeCx}; +use crate::{PatCx, PrivateUninhabitedField}; use self::Constructor::*; @@ -21,7 +21,7 @@ impl PatId { } /// A pattern with an index denoting which field it corresponds to. -pub struct IndexedPat { +pub struct IndexedPat { pub idx: usize, pub pat: DeconstructedPat, } @@ -29,7 +29,7 @@ pub struct IndexedPat { /// Values and patterns can be represented as a constructor applied to some fields. This represents /// a pattern in this form. A `DeconstructedPat` will almost always come from user input; the only /// exception are some `Wildcard`s introduced during pattern lowering. -pub struct DeconstructedPat { +pub struct DeconstructedPat { ctor: Constructor, fields: Vec>, /// The number of fields in this pattern. E.g. if the pattern is `SomeStruct { field12: true, .. @@ -43,7 +43,7 @@ pub struct DeconstructedPat { pub(crate) uid: PatId, } -impl DeconstructedPat { +impl DeconstructedPat { pub fn new( ctor: Constructor, fields: Vec>, @@ -136,7 +136,7 @@ impl DeconstructedPat { } /// This is best effort and not good enough for a `Display` impl. -impl fmt::Debug for DeconstructedPat { +impl fmt::Debug for DeconstructedPat { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let pat = self; let mut first = true; @@ -219,14 +219,14 @@ impl fmt::Debug for DeconstructedPat { /// algorithm. Do not use `Wild` to represent a wildcard pattern comping from user input. /// /// This is morally `Option<&'p DeconstructedPat>` where `None` is interpreted as a wildcard. -pub(crate) enum PatOrWild<'p, Cx: TypeCx> { +pub(crate) enum PatOrWild<'p, Cx: PatCx> { /// A non-user-provided wildcard, created during specialization. Wild, /// A user-provided pattern. Pat(&'p DeconstructedPat), } -impl<'p, Cx: TypeCx> Clone for PatOrWild<'p, Cx> { +impl<'p, Cx: PatCx> Clone for PatOrWild<'p, Cx> { fn clone(&self) -> Self { match self { PatOrWild::Wild => PatOrWild::Wild, @@ -235,9 +235,9 @@ impl<'p, Cx: TypeCx> Clone for PatOrWild<'p, Cx> { } } -impl<'p, Cx: TypeCx> Copy for PatOrWild<'p, Cx> {} +impl<'p, Cx: PatCx> Copy for PatOrWild<'p, Cx> {} -impl<'p, Cx: TypeCx> PatOrWild<'p, Cx> { +impl<'p, Cx: PatCx> PatOrWild<'p, Cx> { pub(crate) fn as_pat(&self) -> Option<&'p DeconstructedPat> { match self { PatOrWild::Wild => None, @@ -283,7 +283,7 @@ impl<'p, Cx: TypeCx> PatOrWild<'p, Cx> { } } -impl<'p, Cx: TypeCx> fmt::Debug for PatOrWild<'p, Cx> { +impl<'p, Cx: PatCx> fmt::Debug for PatOrWild<'p, Cx> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { PatOrWild::Wild => write!(f, "_"), @@ -295,19 +295,19 @@ impl<'p, Cx: TypeCx> fmt::Debug for PatOrWild<'p, Cx> { /// Same idea as `DeconstructedPat`, except this is a fictitious pattern built up for diagnostics /// purposes. As such they don't use interning and can be cloned. #[derive(Debug)] -pub struct WitnessPat { +pub struct WitnessPat { ctor: Constructor, pub(crate) fields: Vec>, ty: Cx::Ty, } -impl Clone for WitnessPat { +impl Clone for WitnessPat { fn clone(&self) -> Self { Self { ctor: self.ctor.clone(), fields: self.fields.clone(), ty: self.ty.clone() } } } -impl WitnessPat { +impl WitnessPat { pub(crate) fn new(ctor: Constructor, fields: Vec, ty: Cx::Ty) -> Self { Self { ctor, fields, ty } } diff --git a/compiler/rustc_pattern_analysis/src/pat_column.rs b/compiler/rustc_pattern_analysis/src/pat_column.rs index ce14fdc364f79..eb4e095c1c662 100644 --- a/compiler/rustc_pattern_analysis/src/pat_column.rs +++ b/compiler/rustc_pattern_analysis/src/pat_column.rs @@ -1,6 +1,6 @@ use crate::constructor::{Constructor, SplitConstructorSet}; use crate::pat::{DeconstructedPat, PatOrWild}; -use crate::{Captures, MatchArm, TypeCx}; +use crate::{Captures, MatchArm, PatCx}; /// A column of patterns in a match, where a column is the intuitive notion of "subpatterns that /// inspect the same subvalue/place". @@ -11,12 +11,12 @@ use crate::{Captures, MatchArm, TypeCx}; /// /// This is not used in the usefulness algorithm; only in lints. #[derive(Debug)] -pub struct PatternColumn<'p, Cx: TypeCx> { +pub struct PatternColumn<'p, Cx: PatCx> { /// This must not contain an or-pattern. `expand_and_push` takes care to expand them. patterns: Vec<&'p DeconstructedPat>, } -impl<'p, Cx: TypeCx> PatternColumn<'p, Cx> { +impl<'p, Cx: PatCx> PatternColumn<'p, Cx> { pub fn new(arms: &[MatchArm<'p, Cx>]) -> Self { let patterns = Vec::with_capacity(arms.len()); let mut column = PatternColumn { patterns }; diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 53a32d3237e6c..6de12f1f6baa9 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -18,7 +18,7 @@ use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT}; use crate::constructor::{ IntRange, MaybeInfiniteInt, OpaqueId, RangeEnd, Slice, SliceKind, VariantVisibility, }; -use crate::{errors, Captures, PrivateUninhabitedField, TypeCx}; +use crate::{errors, Captures, PatCx, PrivateUninhabitedField}; use crate::constructor::Constructor::*; @@ -843,7 +843,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } } -impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> { +impl<'p, 'tcx: 'p> PatCx for RustcMatchCheckCtxt<'p, 'tcx> { type Ty = RevealedTy<'tcx>; type Error = ErrorGuaranteed; type VariantIdx = VariantIdx; diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 495785066ce62..3760db8b68894 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -242,7 +242,7 @@ //! Therefore `usefulness(tp_1, tp_2, tq)` returns the single witness-tuple `[Variant2(Some(true), 0)]`. //! //! -//! Computing the set of constructors for a type is done in [`TypeCx::ctors_for_ty`]. See +//! Computing the set of constructors for a type is done in [`PatCx::ctors_for_ty`]. See //! the following sections for more accurate versions of the algorithm and corresponding links. //! //! @@ -716,7 +716,7 @@ use std::fmt; use crate::constructor::{Constructor, ConstructorSet, IntRange}; use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat}; -use crate::{Captures, MatchArm, PrivateUninhabitedField, TypeCx}; +use crate::{Captures, MatchArm, PatCx, PrivateUninhabitedField}; use self::PlaceValidity::*; @@ -728,7 +728,7 @@ pub fn ensure_sufficient_stack(f: impl FnOnce() -> R) -> R { } /// Context that provides information for usefulness checking. -struct UsefulnessCtxt<'a, Cx: TypeCx> { +struct UsefulnessCtxt<'a, Cx: PatCx> { /// The context for type information. tycx: &'a Cx, /// Collect the patterns found useful during usefulness checking. This is used to lint @@ -738,7 +738,7 @@ struct UsefulnessCtxt<'a, Cx: TypeCx> { complexity_level: usize, } -impl<'a, Cx: TypeCx> UsefulnessCtxt<'a, Cx> { +impl<'a, Cx: PatCx> UsefulnessCtxt<'a, Cx> { fn increase_complexity_level(&mut self, complexity_add: usize) -> Result<(), Cx::Error> { self.complexity_level += complexity_add; if self @@ -752,26 +752,26 @@ impl<'a, Cx: TypeCx> UsefulnessCtxt<'a, Cx> { } /// Context that provides information local to a place under investigation. -struct PlaceCtxt<'a, Cx: TypeCx> { +struct PlaceCtxt<'a, Cx: PatCx> { cx: &'a Cx, /// Type of the place under investigation. ty: &'a Cx::Ty, } -impl<'a, Cx: TypeCx> Copy for PlaceCtxt<'a, Cx> {} -impl<'a, Cx: TypeCx> Clone for PlaceCtxt<'a, Cx> { +impl<'a, Cx: PatCx> Copy for PlaceCtxt<'a, Cx> {} +impl<'a, Cx: PatCx> Clone for PlaceCtxt<'a, Cx> { fn clone(&self) -> Self { Self { cx: self.cx, ty: self.ty } } } -impl<'a, Cx: TypeCx> fmt::Debug for PlaceCtxt<'a, Cx> { +impl<'a, Cx: PatCx> fmt::Debug for PlaceCtxt<'a, Cx> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_struct("PlaceCtxt").field("ty", self.ty).finish() } } -impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> { +impl<'a, Cx: PatCx> PlaceCtxt<'a, Cx> { fn ctor_arity(&self, ctor: &Constructor) -> usize { self.cx.ctor_arity(ctor, self.ty) } @@ -802,7 +802,7 @@ impl PlaceValidity { /// /// Pending further opsem decisions, the current behavior is: validity is preserved, except /// inside `&` and union fields where validity is reset to `MaybeInvalid`. - fn specialize(self, ctor: &Constructor) -> Self { + fn specialize(self, ctor: &Constructor) -> Self { // We preserve validity except when we go inside a reference or a union field. if matches!(ctor, Constructor::Ref | Constructor::UnionField) { // Validity of `x: &T` does not imply validity of `*x: T`. @@ -825,7 +825,7 @@ impl fmt::Display for PlaceValidity { /// Data about a place under investigation. Its methods contain a lot of the logic used to analyze /// the constructors in the matrix. -struct PlaceInfo { +struct PlaceInfo { /// The type of the place. ty: Cx::Ty, /// Whether the place is a private uninhabited field. If so we skip this field during analysis @@ -837,7 +837,7 @@ struct PlaceInfo { is_scrutinee: bool, } -impl PlaceInfo { +impl PlaceInfo { /// Given a constructor for the current place, we return one `PlaceInfo` for each field of the /// constructor. fn specialize<'a>( @@ -932,7 +932,7 @@ impl PlaceInfo { } } -impl Clone for PlaceInfo { +impl Clone for PlaceInfo { fn clone(&self) -> Self { Self { ty: self.ty.clone(), @@ -947,7 +947,7 @@ impl Clone for PlaceInfo { // The three lifetimes are: // - 'p coming from the input // - Cx global compilation context -struct PatStack<'p, Cx: TypeCx> { +struct PatStack<'p, Cx: PatCx> { // Rows of len 1 are very common, which is why `SmallVec[_; 2]` works well. pats: SmallVec<[PatOrWild<'p, Cx>; 2]>, /// Sometimes we know that as far as this row is concerned, the current case is already handled @@ -956,13 +956,13 @@ struct PatStack<'p, Cx: TypeCx> { relevant: bool, } -impl<'p, Cx: TypeCx> Clone for PatStack<'p, Cx> { +impl<'p, Cx: PatCx> Clone for PatStack<'p, Cx> { fn clone(&self) -> Self { Self { pats: self.pats.clone(), relevant: self.relevant } } } -impl<'p, Cx: TypeCx> PatStack<'p, Cx> { +impl<'p, Cx: PatCx> PatStack<'p, Cx> { fn from_pattern(pat: &'p DeconstructedPat) -> Self { PatStack { pats: smallvec![PatOrWild::Pat(pat)], relevant: true } } @@ -1022,7 +1022,7 @@ impl<'p, Cx: TypeCx> PatStack<'p, Cx> { } } -impl<'p, Cx: TypeCx> fmt::Debug for PatStack<'p, Cx> { +impl<'p, Cx: PatCx> fmt::Debug for PatStack<'p, Cx> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // We pretty-print similarly to the `Debug` impl of `Matrix`. write!(f, "+")?; @@ -1035,7 +1035,7 @@ impl<'p, Cx: TypeCx> fmt::Debug for PatStack<'p, Cx> { /// A row of the matrix. #[derive(Clone)] -struct MatrixRow<'p, Cx: TypeCx> { +struct MatrixRow<'p, Cx: PatCx> { // The patterns in the row. pats: PatStack<'p, Cx>, /// Whether the original arm had a guard. This is inherited when specializing. @@ -1055,7 +1055,7 @@ struct MatrixRow<'p, Cx: TypeCx> { intersects: BitSet, } -impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> { +impl<'p, Cx: PatCx> MatrixRow<'p, Cx> { fn is_empty(&self) -> bool { self.pats.is_empty() } @@ -1104,7 +1104,7 @@ impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> { } } -impl<'p, Cx: TypeCx> fmt::Debug for MatrixRow<'p, Cx> { +impl<'p, Cx: PatCx> fmt::Debug for MatrixRow<'p, Cx> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.pats.fmt(f) } @@ -1121,7 +1121,7 @@ impl<'p, Cx: TypeCx> fmt::Debug for MatrixRow<'p, Cx> { /// specializing `(,)` and `Some` on a pattern of type `(Option, bool)`, the first column of /// the matrix will correspond to `scrutinee.0.Some.0` and the second column to `scrutinee.1`. #[derive(Clone)] -struct Matrix<'p, Cx: TypeCx> { +struct Matrix<'p, Cx: PatCx> { /// Vector of rows. The rows must form a rectangular 2D array. Moreover, all the patterns of /// each column must have the same type. Each column corresponds to a place within the /// scrutinee. @@ -1134,7 +1134,7 @@ struct Matrix<'p, Cx: TypeCx> { wildcard_row_is_relevant: bool, } -impl<'p, Cx: TypeCx> Matrix<'p, Cx> { +impl<'p, Cx: PatCx> Matrix<'p, Cx> { /// Pushes a new row to the matrix. If the row starts with an or-pattern, this recursively /// expands it. Internal method, prefer [`Matrix::new`]. fn expand_and_push(&mut self, mut row: MatrixRow<'p, Cx>) { @@ -1256,7 +1256,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { /// + _ + [_, _, tail @ ..] + /// | ✓ | ? | // column validity /// ``` -impl<'p, Cx: TypeCx> fmt::Debug for Matrix<'p, Cx> { +impl<'p, Cx: PatCx> fmt::Debug for Matrix<'p, Cx> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "\n")?; @@ -1347,15 +1347,15 @@ impl<'p, Cx: TypeCx> fmt::Debug for Matrix<'p, Cx> { /// /// See the top of the file for more detailed explanations and examples. #[derive(Debug)] -struct WitnessStack(Vec>); +struct WitnessStack(Vec>); -impl Clone for WitnessStack { +impl Clone for WitnessStack { fn clone(&self) -> Self { Self(self.0.clone()) } } -impl WitnessStack { +impl WitnessStack { /// Asserts that the witness contains a single pattern, and returns it. fn single_pattern(self) -> WitnessPat { assert_eq!(self.0.len(), 1); @@ -1400,15 +1400,15 @@ impl WitnessStack { /// Just as the `Matrix` starts with a single column, by the end of the algorithm, this has a single /// column, which contains the patterns that are missing for the match to be exhaustive. #[derive(Debug)] -struct WitnessMatrix(Vec>); +struct WitnessMatrix(Vec>); -impl Clone for WitnessMatrix { +impl Clone for WitnessMatrix { fn clone(&self) -> Self { Self(self.0.clone()) } } -impl WitnessMatrix { +impl WitnessMatrix { /// New matrix with no witnesses. fn empty() -> Self { WitnessMatrix(Vec::new()) @@ -1482,7 +1482,7 @@ impl WitnessMatrix { /// /// We can however get false negatives because exhaustiveness does not explore all cases. See the /// section on relevancy at the top of the file. -fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>( +fn collect_overlapping_range_endpoints<'p, Cx: PatCx>( cx: &Cx, overlap_range: IntRange, matrix: &Matrix<'p, Cx>, @@ -1541,7 +1541,7 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>( } /// Collect ranges that have a singleton gap between them. -fn collect_non_contiguous_range_endpoints<'p, Cx: TypeCx>( +fn collect_non_contiguous_range_endpoints<'p, Cx: PatCx>( cx: &Cx, gap_range: &IntRange, matrix: &Matrix<'p, Cx>, @@ -1582,7 +1582,7 @@ fn collect_non_contiguous_range_endpoints<'p, Cx: TypeCx>( /// (using `apply_constructor` and by updating `row.useful` for each parent row). /// This is all explained at the top of the file. #[instrument(level = "debug", skip(mcx), ret)] -fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( +fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: PatCx>( mcx: &mut UsefulnessCtxt<'a, Cx>, matrix: &mut Matrix<'p, Cx>, ) -> Result, Cx::Error> { @@ -1679,7 +1679,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( /// Indicates whether or not a given arm is useful. #[derive(Clone, Debug)] -pub enum Usefulness<'p, Cx: TypeCx> { +pub enum Usefulness<'p, Cx: PatCx> { /// The arm is useful. This additionally carries a set of or-pattern branches that have been /// found to be redundant despite the overall arm being useful. Used only in the presence of /// or-patterns, otherwise it stays empty. @@ -1690,11 +1690,11 @@ pub enum Usefulness<'p, Cx: TypeCx> { } /// Report whether this pattern was found useful, and its subpatterns that were not useful if any. -fn collect_pattern_usefulness<'p, Cx: TypeCx>( +fn collect_pattern_usefulness<'p, Cx: PatCx>( useful_subpatterns: &FxHashSet, pat: &'p DeconstructedPat, ) -> Usefulness<'p, Cx> { - fn pat_is_useful<'p, Cx: TypeCx>( + fn pat_is_useful<'p, Cx: PatCx>( useful_subpatterns: &FxHashSet, pat: &'p DeconstructedPat, ) -> bool { @@ -1732,7 +1732,7 @@ fn collect_pattern_usefulness<'p, Cx: TypeCx>( } /// The output of checking a match for exhaustiveness and arm usefulness. -pub struct UsefulnessReport<'p, Cx: TypeCx> { +pub struct UsefulnessReport<'p, Cx: PatCx> { /// For each arm of the input, whether that arm is useful after the arms above it. pub arm_usefulness: Vec<(MatchArm<'p, Cx>, Usefulness<'p, Cx>)>, /// If the match is exhaustive, this is empty. If not, this contains witnesses for the lack of @@ -1742,7 +1742,7 @@ pub struct UsefulnessReport<'p, Cx: TypeCx> { /// Computes whether a match is exhaustive and which of its arms are useful. #[instrument(skip(tycx, arms), level = "debug")] -pub fn compute_match_usefulness<'p, Cx: TypeCx>( +pub fn compute_match_usefulness<'p, Cx: PatCx>( tycx: &Cx, arms: &[MatchArm<'p, Cx>], scrut_ty: Cx::Ty, From f27540697e7f17b77dd2fae9fe5d9df8a5eb24c0 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 13 Mar 2024 14:07:44 +0100 Subject: [PATCH 31/32] Rename `RustcMatchCheckCtxt` -> `RustcPatCtxt` --- compiler/rustc_mir_build/src/errors.rs | 4 +- .../src/thir/pattern/check_match.rs | 29 ++++++------- compiler/rustc_pattern_analysis/src/errors.rs | 4 +- compiler/rustc_pattern_analysis/src/lib.rs | 2 +- compiler/rustc_pattern_analysis/src/lints.rs | 12 +++--- compiler/rustc_pattern_analysis/src/rustc.rs | 41 ++++++++----------- 6 files changed, 42 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 2b4c65418577c..848da56f98168 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -6,7 +6,7 @@ use rustc_errors::{ }; use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic}; use rustc_middle::ty::{self, Ty}; -use rustc_pattern_analysis::{errors::Uncovered, rustc::RustcMatchCheckCtxt}; +use rustc_pattern_analysis::{errors::Uncovered, rustc::RustcPatCtxt}; use rustc_span::symbol::Symbol; use rustc_span::Span; @@ -455,7 +455,7 @@ pub enum UnusedUnsafeEnclosing { } pub(crate) struct NonExhaustivePatternsTypeNotEmpty<'p, 'tcx, 'm> { - pub cx: &'m RustcMatchCheckCtxt<'p, 'tcx>, + pub cx: &'m RustcPatCtxt<'p, 'tcx>, pub expr_span: Span, pub span: Span, pub ty: Ty<'tcx>, diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index f395bb44f5745..20d3b3f98ce31 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -1,7 +1,7 @@ use rustc_pattern_analysis::errors::Uncovered; use rustc_pattern_analysis::rustc::{ - Constructor, DeconstructedPat, MatchArm, RustcMatchCheckCtxt as MatchCheckCtxt, Usefulness, - UsefulnessReport, WitnessPat, + Constructor, DeconstructedPat, MatchArm, RustcPatCtxt as PatCtxt, Usefulness, UsefulnessReport, + WitnessPat, }; use crate::errors::*; @@ -276,7 +276,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { fn lower_pattern( &mut self, - cx: &MatchCheckCtxt<'p, 'tcx>, + cx: &PatCtxt<'p, 'tcx>, pat: &'p Pat<'tcx>, ) -> Result<&'p DeconstructedPat<'p, 'tcx>, ErrorGuaranteed> { if let Err(err) = pat.pat_error_reported() { @@ -375,7 +375,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { whole_match_span: Option, scrutinee: Option<&Expr<'tcx>>, scrut_span: Span, - ) -> MatchCheckCtxt<'p, 'tcx> { + ) -> PatCtxt<'p, 'tcx> { let refutable = match refutability { Irrefutable => false, Refutable => true, @@ -384,7 +384,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { // require validity. let known_valid_scrutinee = scrutinee.map(|scrut| self.is_known_valid_scrutinee(scrut)).unwrap_or(true); - MatchCheckCtxt { + PatCtxt { tcx: self.tcx, typeck_results: self.typeck_results, param_env: self.param_env, @@ -400,7 +400,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { fn analyze_patterns( &mut self, - cx: &MatchCheckCtxt<'p, 'tcx>, + cx: &PatCtxt<'p, 'tcx>, arms: &[MatchArm<'p, 'tcx>], scrut_ty: Ty<'tcx>, ) -> Result, ErrorGuaranteed> { @@ -584,7 +584,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { pat: &'p Pat<'tcx>, refutability: RefutableFlag, scrut: Option<&Expr<'tcx>>, - ) -> Result<(MatchCheckCtxt<'p, 'tcx>, UsefulnessReport<'p, 'tcx>), ErrorGuaranteed> { + ) -> Result<(PatCtxt<'p, 'tcx>, UsefulnessReport<'p, 'tcx>), ErrorGuaranteed> { let cx = self.new_cx(refutability, None, scrut, pat.span); let pat = self.lower_pattern(&cx, pat)?; let arms = [MatchArm { pat, arm_data: self.lint_level, has_guard: false }]; @@ -849,7 +849,7 @@ fn check_for_bindings_named_same_as_variants( /// Check that never patterns are only used on inhabited types. fn check_never_pattern<'tcx>( - cx: &MatchCheckCtxt<'_, 'tcx>, + cx: &PatCtxt<'_, 'tcx>, pat: &Pat<'tcx>, ) -> Result<(), ErrorGuaranteed> { if let PatKind::Never = pat.kind { @@ -884,7 +884,7 @@ fn report_irrefutable_let_patterns( /// Report unreachable arms, if any. fn report_unreachable_pattern<'p, 'tcx>( - cx: &MatchCheckCtxt<'p, 'tcx>, + cx: &PatCtxt<'p, 'tcx>, hir_id: HirId, span: Span, catchall: Option, @@ -898,10 +898,7 @@ fn report_unreachable_pattern<'p, 'tcx>( } /// Report unreachable arms, if any. -fn report_arm_reachability<'p, 'tcx>( - cx: &MatchCheckCtxt<'p, 'tcx>, - report: &UsefulnessReport<'p, 'tcx>, -) { +fn report_arm_reachability<'p, 'tcx>(cx: &PatCtxt<'p, 'tcx>, report: &UsefulnessReport<'p, 'tcx>) { let mut catchall = None; for (arm, is_useful) in report.arm_usefulness.iter() { if matches!(is_useful, Usefulness::Redundant) { @@ -926,7 +923,7 @@ fn pat_is_catchall(pat: &DeconstructedPat<'_, '_>) -> bool { /// Report that a match is not exhaustive. fn report_non_exhaustive_match<'p, 'tcx>( - cx: &MatchCheckCtxt<'p, 'tcx>, + cx: &PatCtxt<'p, 'tcx>, thir: &Thir<'tcx>, scrut_ty: Ty<'tcx>, sp: Span, @@ -1126,7 +1123,7 @@ fn report_non_exhaustive_match<'p, 'tcx>( } fn joined_uncovered_patterns<'p, 'tcx>( - cx: &MatchCheckCtxt<'p, 'tcx>, + cx: &PatCtxt<'p, 'tcx>, witnesses: &[WitnessPat<'p, 'tcx>], ) -> String { const LIMIT: usize = 3; @@ -1147,7 +1144,7 @@ fn joined_uncovered_patterns<'p, 'tcx>( } fn collect_non_exhaustive_tys<'tcx>( - cx: &MatchCheckCtxt<'_, 'tcx>, + cx: &PatCtxt<'_, 'tcx>, pat: &WitnessPat<'_, 'tcx>, non_exhaustive_tys: &mut FxIndexSet>, ) { diff --git a/compiler/rustc_pattern_analysis/src/errors.rs b/compiler/rustc_pattern_analysis/src/errors.rs index 21a61d46ccb7b..75b7b7c8f6784 100644 --- a/compiler/rustc_pattern_analysis/src/errors.rs +++ b/compiler/rustc_pattern_analysis/src/errors.rs @@ -4,7 +4,7 @@ use rustc_middle::thir::Pat; use rustc_middle::ty::Ty; use rustc_span::Span; -use crate::rustc::{RustcMatchCheckCtxt, WitnessPat}; +use crate::rustc::{RustcPatCtxt, WitnessPat}; #[derive(Subdiagnostic)] #[label(pattern_analysis_uncovered)] @@ -21,7 +21,7 @@ pub struct Uncovered<'tcx> { impl<'tcx> Uncovered<'tcx> { pub fn new<'p>( span: Span, - cx: &RustcMatchCheckCtxt<'p, 'tcx>, + cx: &RustcPatCtxt<'p, 'tcx>, witnesses: Vec>, ) -> Self where diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index ff085d2a422d3..5c57c990323c8 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -173,7 +173,7 @@ impl<'p, Cx: PatCx> Copy for MatchArm<'p, Cx> {} /// useful, and runs some lints. #[cfg(feature = "rustc")] pub fn analyze_match<'p, 'tcx>( - tycx: &rustc::RustcMatchCheckCtxt<'p, 'tcx>, + tycx: &rustc::RustcPatCtxt<'p, 'tcx>, arms: &[rustc::MatchArm<'p, 'tcx>], scrut_ty: Ty<'tcx>, pattern_complexity_limit: Option, diff --git a/compiler/rustc_pattern_analysis/src/lints.rs b/compiler/rustc_pattern_analysis/src/lints.rs index 072a8e4bfe5a0..3ca5ebdb0dd13 100644 --- a/compiler/rustc_pattern_analysis/src/lints.rs +++ b/compiler/rustc_pattern_analysis/src/lints.rs @@ -4,15 +4,15 @@ use rustc_span::ErrorGuaranteed; use crate::constructor::Constructor; use crate::errors::{NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered}; use crate::pat_column::PatternColumn; -use crate::rustc::{RevealedTy, RustcMatchCheckCtxt, WitnessPat}; +use crate::rustc::{RevealedTy, RustcPatCtxt, WitnessPat}; use crate::MatchArm; /// Traverse the patterns to collect any variants of a non_exhaustive enum that fail to be mentioned /// in a given column. #[instrument(level = "debug", skip(cx), ret)] fn collect_nonexhaustive_missing_variants<'p, 'tcx>( - cx: &RustcMatchCheckCtxt<'p, 'tcx>, - column: &PatternColumn<'p, RustcMatchCheckCtxt<'p, 'tcx>>, + cx: &RustcPatCtxt<'p, 'tcx>, + column: &PatternColumn<'p, RustcPatCtxt<'p, 'tcx>>, ) -> Result>, ErrorGuaranteed> { let Some(&ty) = column.head_ty() else { return Ok(Vec::new()); @@ -57,9 +57,9 @@ fn collect_nonexhaustive_missing_variants<'p, 'tcx>( } pub(crate) fn lint_nonexhaustive_missing_variants<'p, 'tcx>( - rcx: &RustcMatchCheckCtxt<'p, 'tcx>, - arms: &[MatchArm<'p, RustcMatchCheckCtxt<'p, 'tcx>>], - pat_column: &PatternColumn<'p, RustcMatchCheckCtxt<'p, 'tcx>>, + rcx: &RustcPatCtxt<'p, 'tcx>, + arms: &[MatchArm<'p, RustcPatCtxt<'p, 'tcx>>], + pat_column: &PatternColumn<'p, RustcPatCtxt<'p, 'tcx>>, scrut_ty: RevealedTy<'tcx>, ) -> Result<(), ErrorGuaranteed> { if !matches!( diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 6de12f1f6baa9..d00faadccb095 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -23,15 +23,14 @@ use crate::{errors, Captures, PatCx, PrivateUninhabitedField}; use crate::constructor::Constructor::*; // Re-export rustc-specific versions of all these types. -pub type Constructor<'p, 'tcx> = crate::constructor::Constructor>; -pub type ConstructorSet<'p, 'tcx> = - crate::constructor::ConstructorSet>; -pub type DeconstructedPat<'p, 'tcx> = crate::pat::DeconstructedPat>; -pub type MatchArm<'p, 'tcx> = crate::MatchArm<'p, RustcMatchCheckCtxt<'p, 'tcx>>; -pub type Usefulness<'p, 'tcx> = crate::usefulness::Usefulness<'p, RustcMatchCheckCtxt<'p, 'tcx>>; +pub type Constructor<'p, 'tcx> = crate::constructor::Constructor>; +pub type ConstructorSet<'p, 'tcx> = crate::constructor::ConstructorSet>; +pub type DeconstructedPat<'p, 'tcx> = crate::pat::DeconstructedPat>; +pub type MatchArm<'p, 'tcx> = crate::MatchArm<'p, RustcPatCtxt<'p, 'tcx>>; +pub type Usefulness<'p, 'tcx> = crate::usefulness::Usefulness<'p, RustcPatCtxt<'p, 'tcx>>; pub type UsefulnessReport<'p, 'tcx> = - crate::usefulness::UsefulnessReport<'p, RustcMatchCheckCtxt<'p, 'tcx>>; -pub type WitnessPat<'p, 'tcx> = crate::pat::WitnessPat>; + crate::usefulness::UsefulnessReport<'p, RustcPatCtxt<'p, 'tcx>>; +pub type WitnessPat<'p, 'tcx> = crate::pat::WitnessPat>; /// A type which has gone through `cx.reveal_opaque_ty`, i.e. if it was opaque it was replaced by /// the hidden type if allowed in the current body. This ensures we consistently inspect the hidden @@ -62,7 +61,7 @@ impl<'tcx> RevealedTy<'tcx> { } #[derive(Clone)] -pub struct RustcMatchCheckCtxt<'p, 'tcx: 'p> { +pub struct RustcPatCtxt<'p, 'tcx: 'p> { pub tcx: TyCtxt<'tcx>, pub typeck_results: &'tcx ty::TypeckResults<'tcx>, /// The module in which the match occurs. This is necessary for @@ -87,22 +86,19 @@ pub struct RustcMatchCheckCtxt<'p, 'tcx: 'p> { pub known_valid_scrutinee: bool, } -impl<'p, 'tcx: 'p> fmt::Debug for RustcMatchCheckCtxt<'p, 'tcx> { +impl<'p, 'tcx: 'p> fmt::Debug for RustcPatCtxt<'p, 'tcx> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("RustcMatchCheckCtxt").finish() + f.debug_struct("RustcPatCtxt").finish() } } -impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { +impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { /// Type inference occasionally gives us opaque types in places where corresponding patterns /// have more specific types. To avoid inconsistencies as well as detect opaque uninhabited /// types, we use the corresponding concrete type if possible. #[inline] pub fn reveal_opaque_ty(&self, ty: Ty<'tcx>) -> RevealedTy<'tcx> { - fn reveal_inner<'tcx>( - cx: &RustcMatchCheckCtxt<'_, 'tcx>, - ty: Ty<'tcx>, - ) -> RevealedTy<'tcx> { + fn reveal_inner<'tcx>(cx: &RustcPatCtxt<'_, 'tcx>, ty: Ty<'tcx>) -> RevealedTy<'tcx> { let ty::Alias(ty::Opaque, alias_ty) = *ty.kind() else { bug!() }; if let Some(local_def_id) = alias_ty.def_id.as_local() { let key = ty::OpaqueTypeKey { def_id: local_def_id, args: alias_ty.args }; @@ -199,7 +195,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { + ExactSizeIterator + Captures<'a> { fn reveal_and_alloc<'a, 'tcx>( - cx: &'a RustcMatchCheckCtxt<'_, 'tcx>, + cx: &'a RustcPatCtxt<'_, 'tcx>, iter: impl Iterator>, ) -> &'a [(RevealedTy<'tcx>, PrivateUninhabitedField)] { cx.dropless_arena.alloc_from_iter( @@ -218,7 +214,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { reveal_and_alloc(cx, once(args.type_at(0))) } else { let variant = - &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); + &adt.variant(RustcPatCtxt::variant_index_for_adt(&ctor, *adt)); // In the cases of either a `#[non_exhaustive]` field list or a non-public // field, we skip uninhabited fields in order not to reveal the @@ -270,7 +266,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { // patterns. If we're here we can assume this is a box pattern. 1 } else { - let variant_idx = RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt); + let variant_idx = RustcPatCtxt::variant_index_for_adt(&ctor, *adt); adt.variant(variant_idx).fields.len() } } @@ -506,7 +502,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { _ => bug!(), }; let variant = - &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); + &adt.variant(RustcPatCtxt::variant_index_for_adt(&ctor, *adt)); arity = variant.fields.len(); fields = subpatterns .iter() @@ -774,8 +770,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { PatKind::Deref { subpattern: subpatterns.next().unwrap() } } ty::Adt(adt_def, args) => { - let variant_index = - RustcMatchCheckCtxt::variant_index_for_adt(&pat.ctor(), *adt_def); + let variant_index = RustcPatCtxt::variant_index_for_adt(&pat.ctor(), *adt_def); let subpatterns = subpatterns .enumerate() .map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern }) @@ -843,7 +838,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } } -impl<'p, 'tcx: 'p> PatCx for RustcMatchCheckCtxt<'p, 'tcx> { +impl<'p, 'tcx: 'p> PatCx for RustcPatCtxt<'p, 'tcx> { type Ty = RevealedTy<'tcx>; type Error = ErrorGuaranteed; type VariantIdx = VariantIdx; From c4236785c72fdf04176716393c910b1fb011d15f Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 13 Mar 2024 14:17:11 +0100 Subject: [PATCH 32/32] Remove `MaybeInfiniteInt::JustAfterMax` It was inherited from before half-open ranges, but it doesn't pull its weight anymore. We lose a tiny bit of diagnostic precision. --- .../rustc_pattern_analysis/src/constructor.rs | 13 +++++-------- compiler/rustc_pattern_analysis/src/rustc.rs | 2 +- .../half-open-range-pats-exhaustive-fail.stderr | 16 ++++++++-------- .../integer-ranges/exhaustiveness.stderr | 8 ++++---- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 2d55785cd0692..ab75b2fef9970 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -195,8 +195,6 @@ pub enum MaybeInfiniteInt { /// Encoded value. DO NOT CONSTRUCT BY HAND; use `new_finite_{int,uint}`. #[non_exhaustive] Finite(u128), - /// The integer after `u128::MAX`. We need it to represent `x..=u128::MAX` as an exclusive range. - JustAfterMax, PosInfinity, } @@ -232,18 +230,18 @@ impl MaybeInfiniteInt { pub fn minus_one(self) -> Option { match self { Finite(n) => n.checked_sub(1).map(Finite), - JustAfterMax => Some(Finite(u128::MAX)), x => Some(x), } } - /// Note: this will not turn a finite value into an infinite one or vice-versa. + /// Note: this will turn `u128::MAX` into `PosInfinity`. This means `plus_one` and `minus_one` + /// are not strictly inverses, but that poses no problem in our use of them. + /// this will not turn a finite value into an infinite one or vice-versa. pub fn plus_one(self) -> Option { match self { Finite(n) => match n.checked_add(1) { Some(m) => Some(Finite(m)), - None => Some(JustAfterMax), + None => Some(PosInfinity), }, - JustAfterMax => None, x => Some(x), } } @@ -277,8 +275,7 @@ impl IntRange { } /// Construct a range with these boundaries. - /// `lo` must not be `PosInfinity` or `JustAfterMax`. `hi` must not be `NegInfinity`. - /// If `end` is `Included`, `hi` must also not be `JustAfterMax`. + /// `lo` must not be `PosInfinity`. `hi` must not be `NegInfinity`. #[inline] pub fn from_range(lo: MaybeInfiniteInt, mut hi: MaybeInfiniteInt, end: RangeEnd) -> IntRange { if end == RangeEnd::Included { diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 53a32d3237e6c..f4a3983338519 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -710,7 +710,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { None => PatRangeBoundary::PosInfinity, } } - JustAfterMax | PosInfinity => PatRangeBoundary::PosInfinity, + PosInfinity => PatRangeBoundary::PosInfinity, } } diff --git a/tests/ui/half-open-range-patterns/half-open-range-pats-exhaustive-fail.stderr b/tests/ui/half-open-range-patterns/half-open-range-pats-exhaustive-fail.stderr index 6b20a820b7302..bb4c2a4c52353 100644 --- a/tests/ui/half-open-range-patterns/half-open-range-pats-exhaustive-fail.stderr +++ b/tests/ui/half-open-range-patterns/half-open-range-pats-exhaustive-fail.stderr @@ -394,17 +394,17 @@ help: ensure that all possible cases are being handled by adding a match arm wit LL | match $s { $($t)+ => {}, u128::MAX => todo!() } | ++++++++++++++++++++++ -error[E0004]: non-exhaustive patterns: `340282366920938463463374607431768211454_u128..=u128::MAX` not covered +error[E0004]: non-exhaustive patterns: `340282366920938463463374607431768211454_u128..` not covered --> $DIR/half-open-range-pats-exhaustive-fail.rs:93:12 | LL | m!(0, ..ALMOST_MAX); - | ^ pattern `340282366920938463463374607431768211454_u128..=u128::MAX` not covered + | ^ pattern `340282366920938463463374607431768211454_u128..` not covered | = note: the matched value is of type `u128` help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown | -LL | match $s { $($t)+ => {}, 340282366920938463463374607431768211454_u128..=u128::MAX => todo!() } - | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +LL | match $s { $($t)+ => {}, 340282366920938463463374607431768211454_u128.. => todo!() } + | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ error[E0004]: non-exhaustive patterns: `0_u128` not covered --> $DIR/half-open-range-pats-exhaustive-fail.rs:94:12 @@ -754,17 +754,17 @@ help: ensure that all possible cases are being handled by adding a match arm wit LL | match $s { $($t)+ => {}, i128::MAX => todo!() } | ++++++++++++++++++++++ -error[E0004]: non-exhaustive patterns: `170141183460469231731687303715884105726_i128..=i128::MAX` not covered +error[E0004]: non-exhaustive patterns: `170141183460469231731687303715884105726_i128..` not covered --> $DIR/half-open-range-pats-exhaustive-fail.rs:161:12 | LL | m!(0, ..ALMOST_MAX); - | ^ pattern `170141183460469231731687303715884105726_i128..=i128::MAX` not covered + | ^ pattern `170141183460469231731687303715884105726_i128..` not covered | = note: the matched value is of type `i128` help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown | -LL | match $s { $($t)+ => {}, 170141183460469231731687303715884105726_i128..=i128::MAX => todo!() } - | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +LL | match $s { $($t)+ => {}, 170141183460469231731687303715884105726_i128.. => todo!() } + | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ error[E0004]: non-exhaustive patterns: `i128::MIN` not covered --> $DIR/half-open-range-pats-exhaustive-fail.rs:162:12 diff --git a/tests/ui/pattern/usefulness/integer-ranges/exhaustiveness.stderr b/tests/ui/pattern/usefulness/integer-ranges/exhaustiveness.stderr index 90d0fd7483a99..68976c1b0576f 100644 --- a/tests/ui/pattern/usefulness/integer-ranges/exhaustiveness.stderr +++ b/tests/ui/pattern/usefulness/integer-ranges/exhaustiveness.stderr @@ -107,17 +107,17 @@ help: ensure that all possible cases are being handled by adding a match arm wit LL | match $s { $($t)+ => {}, u128::MAX => todo!() } | ++++++++++++++++++++++ -error[E0004]: non-exhaustive patterns: `5_u128..=u128::MAX` not covered +error[E0004]: non-exhaustive patterns: `5_u128..` not covered --> $DIR/exhaustiveness.rs:61:8 | LL | m!(0u128, 0..=4); - | ^^^^^ pattern `5_u128..=u128::MAX` not covered + | ^^^^^ pattern `5_u128..` not covered | = note: the matched value is of type `u128` help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown | -LL | match $s { $($t)+ => {}, 5_u128..=u128::MAX => todo!() } - | +++++++++++++++++++++++++++++++ +LL | match $s { $($t)+ => {}, 5_u128.. => todo!() } + | +++++++++++++++++++++ error[E0004]: non-exhaustive patterns: `0_u128` not covered --> $DIR/exhaustiveness.rs:62:8