From bf62d5913a702754d46a0e9210fcf608deba63af Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 9 Feb 2024 16:16:22 +1100 Subject: [PATCH 01/33] 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 02/33] 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 03/33] 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 04/33] 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 05/33] 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 06/33] 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 07/33] 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 408c0ea2162b9892540a5b3916ddcac7713de8c3 Mon Sep 17 00:00:00 2001 From: Antoine PLASKOWSKI Date: Tue, 25 Jul 2023 05:59:22 +0200 Subject: [PATCH 08/33] unix time module now return result --- library/std/src/sys/pal/unix/fs.rs | 30 +++++++-------- library/std/src/sys/pal/unix/time.rs | 56 ++++++++++------------------ 2 files changed, 35 insertions(+), 51 deletions(-) diff --git a/library/std/src/sys/pal/unix/fs.rs b/library/std/src/sys/pal/unix/fs.rs index c75323ef7757a..87646bfdfe73a 100644 --- a/library/std/src/sys/pal/unix/fs.rs +++ b/library/std/src/sys/pal/unix/fs.rs @@ -463,15 +463,15 @@ impl FileAttr { #[cfg(target_os = "netbsd")] impl FileAttr { pub fn modified(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_mtime as i64, self.stat.st_mtimensec as i64)) + SystemTime::new(self.stat.st_mtime as i64, self.stat.st_mtimensec as i64) } pub fn accessed(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_atime as i64, self.stat.st_atimensec as i64)) + SystemTime::new(self.stat.st_atime as i64, self.stat.st_atimensec as i64) } pub fn created(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_birthtime as i64, self.stat.st_birthtimensec as i64)) + SystemTime::new(self.stat.st_birthtime as i64, self.stat.st_birthtimensec as i64) } } @@ -503,16 +503,16 @@ impl FileAttr { #[cfg(target_pointer_width = "32")] cfg_has_statx! { if let Some(mtime) = self.stx_mtime() { - return Ok(SystemTime::new(mtime.tv_sec, mtime.tv_nsec as i64)); + return SystemTime::new(mtime.tv_sec, mtime.tv_nsec as i64); } } - Ok(SystemTime::new(self.stat.st_mtime as i64, self.stat.st_mtime_nsec as i64)) + SystemTime::new(self.stat.st_mtime as i64, self.stat.st_mtime_nsec as i64) } #[cfg(any(target_os = "vxworks", target_os = "espidf", target_os = "vita"))] pub fn modified(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_mtime as i64, 0)) + SystemTime::new(self.stat.st_mtime as i64, 0) } #[cfg(any(target_os = "horizon", target_os = "hurd"))] @@ -531,16 +531,16 @@ impl FileAttr { #[cfg(target_pointer_width = "32")] cfg_has_statx! { if let Some(atime) = self.stx_atime() { - return Ok(SystemTime::new(atime.tv_sec, atime.tv_nsec as i64)); + return SystemTime::new(atime.tv_sec, atime.tv_nsec as i64); } } - Ok(SystemTime::new(self.stat.st_atime as i64, self.stat.st_atime_nsec as i64)) + SystemTime::new(self.stat.st_atime as i64, self.stat.st_atime_nsec as i64) } #[cfg(any(target_os = "vxworks", target_os = "espidf", target_os = "vita"))] pub fn accessed(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_atime as i64, 0)) + SystemTime::new(self.stat.st_atime as i64, 0) } #[cfg(any(target_os = "horizon", target_os = "hurd"))] @@ -557,7 +557,7 @@ impl FileAttr { target_os = "watchos", ))] pub fn created(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_birthtime as i64, self.stat.st_birthtime_nsec as i64)) + SystemTime::new(self.stat.st_birthtime as i64, self.stat.st_birthtime_nsec as i64) } #[cfg(not(any( @@ -573,7 +573,7 @@ impl FileAttr { cfg_has_statx! { if let Some(ext) = &self.statx_extra_fields { return if (ext.stx_mask & libc::STATX_BTIME) != 0 { - Ok(SystemTime::new(ext.stx_btime.tv_sec, ext.stx_btime.tv_nsec as i64)) + SystemTime::new(ext.stx_btime.tv_sec, ext.stx_btime.tv_nsec as i64) } else { Err(io::const_io_error!( io::ErrorKind::Uncategorized, @@ -592,22 +592,22 @@ impl FileAttr { #[cfg(target_os = "vita")] pub fn created(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_ctime as i64, 0)) + SystemTime::new(self.stat.st_ctime as i64, 0) } } #[cfg(target_os = "nto")] impl FileAttr { pub fn modified(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_mtim.tv_sec, self.stat.st_mtim.tv_nsec)) + SystemTime::new(self.stat.st_mtim.tv_sec, self.stat.st_mtim.tv_nsec) } pub fn accessed(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_atim.tv_sec, self.stat.st_atim.tv_nsec)) + SystemTime::new(self.stat.st_atim.tv_sec, self.stat.st_atim.tv_nsec) } pub fn created(&self) -> io::Result { - Ok(SystemTime::new(self.stat.st_ctim.tv_sec, self.stat.st_ctim.tv_nsec)) + SystemTime::new(self.stat.st_ctim.tv_sec, self.stat.st_ctim.tv_nsec) } } diff --git a/library/std/src/sys/pal/unix/time.rs b/library/std/src/sys/pal/unix/time.rs index f62eb828ee5d4..911cf29843e7b 100644 --- a/library/std/src/sys/pal/unix/time.rs +++ b/library/std/src/sys/pal/unix/time.rs @@ -1,5 +1,5 @@ -use crate::fmt; use crate::time::Duration; +use crate::{fmt, io}; const NSEC_PER_SEC: u64 = 1_000_000_000; pub const UNIX_EPOCH: SystemTime = SystemTime { t: Timespec::zero() }; @@ -34,8 +34,8 @@ pub(crate) struct Timespec { impl SystemTime { #[cfg_attr(any(target_os = "horizon", target_os = "hurd"), allow(unused))] - pub fn new(tv_sec: i64, tv_nsec: i64) -> SystemTime { - SystemTime { t: Timespec::new(tv_sec, tv_nsec) } + pub fn new(tv_sec: i64, tv_nsec: i64) -> Result { + Ok(SystemTime { t: Timespec::new(tv_sec, tv_nsec)? }) } pub fn now() -> SystemTime { @@ -55,12 +55,6 @@ impl SystemTime { } } -impl From for SystemTime { - fn from(t: libc::timespec) -> SystemTime { - SystemTime { t: Timespec::from(t) } - } -} - impl fmt::Debug for SystemTime { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("SystemTime") @@ -71,11 +65,15 @@ impl fmt::Debug for SystemTime { } impl Timespec { + const unsafe fn new_unchecked(tv_sec: i64, tv_nsec: i64) -> Timespec { + Timespec { tv_sec, tv_nsec: unsafe { Nanoseconds(tv_nsec as u32) } } + } + pub const fn zero() -> Timespec { - Timespec::new(0, 0) + unsafe { Self::new_unchecked(0, 0) } } - const fn new(tv_sec: i64, tv_nsec: i64) -> Timespec { + const fn new(tv_sec: i64, tv_nsec: i64) -> Result { // On Apple OS, dates before epoch are represented differently than on other // Unix platforms: e.g. 1/10th of a second before epoch is represented as `seconds=-1` // and `nanoseconds=100_000_000` on other platforms, but is `seconds=0` and @@ -100,9 +98,11 @@ impl Timespec { } else { (tv_sec, tv_nsec) }; - assert!(tv_nsec >= 0 && tv_nsec < NSEC_PER_SEC as i64); - // SAFETY: The assert above checks tv_nsec is within the valid range - Timespec { tv_sec, tv_nsec: unsafe { Nanoseconds(tv_nsec as u32) } } + if tv_nsec >= 0 && tv_nsec < NSEC_PER_SEC as i64 { + Ok(unsafe { Self::new_unchecked(tv_sec, tv_nsec) }) + } else { + Err(io::const_io_error!(io::ErrorKind::InvalidData, "Invalid timestamp")) + } } pub fn now(clock: libc::clockid_t) -> Timespec { @@ -126,13 +126,15 @@ impl Timespec { if let Some(clock_gettime64) = __clock_gettime64.get() { let mut t = MaybeUninit::uninit(); cvt(unsafe { clock_gettime64(clock, t.as_mut_ptr()) }).unwrap(); - return Timespec::from(unsafe { t.assume_init() }); + let t = unsafe { t.assume_init() }; + return Timespec::new(t.tv_sec as i64, t.tv_nsec as i64).unwrap(); } } let mut t = MaybeUninit::uninit(); cvt(unsafe { libc::clock_gettime(clock, t.as_mut_ptr()) }).unwrap(); - Timespec::from(unsafe { t.assume_init() }) + let t = unsafe { t.assume_init() }; + Timespec::new(t.tv_sec as i64, t.tv_nsec as i64).unwrap() } pub fn sub_timespec(&self, other: &Timespec) -> Result { @@ -178,7 +180,7 @@ impl Timespec { nsec -= NSEC_PER_SEC as u32; secs = secs.checked_add(1)?; } - Some(Timespec::new(secs, nsec.into())) + Some(unsafe { Timespec::new_unchecked(secs, nsec.into()) }) } pub fn checked_sub_duration(&self, other: &Duration) -> Option { @@ -190,7 +192,7 @@ impl Timespec { nsec += NSEC_PER_SEC as i32; secs = secs.checked_sub(1)?; } - Some(Timespec::new(secs, nsec.into())) + Some(unsafe { Timespec::new_unchecked(secs, nsec.into()) }) } #[allow(dead_code)] @@ -226,12 +228,6 @@ impl Timespec { } } -impl From for Timespec { - fn from(t: libc::timespec) -> Timespec { - Timespec::new(t.tv_sec as i64, t.tv_nsec as i64) - } -} - #[cfg(all( target_os = "linux", target_env = "gnu", @@ -260,18 +256,6 @@ impl __timespec64 { } } -#[cfg(all( - target_os = "linux", - target_env = "gnu", - target_pointer_width = "32", - not(target_arch = "riscv32") -))] -impl From<__timespec64> for Timespec { - fn from(t: __timespec64) -> Timespec { - Timespec::new(t.tv_sec, t.tv_nsec.into()) - } -} - #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Instant { t: Timespec, From 71080dd1d45007e9f806bb913cae47531c4dee12 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Sat, 2 Mar 2024 09:55:06 -0500 Subject: [PATCH 09/33] Document how removing a type's field can be bad and what to do instead Related to #119645 --- compiler/rustc_lint_defs/src/builtin.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 94f8bbe2437f8..54b86ec39ab19 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -702,6 +702,20 @@ declare_lint! { /// `PhantomData`. /// /// Otherwise consider removing the unused code. + /// + /// ### Limitations + /// + /// Removing fields that are only used for side-effects and never + /// read will result in behavioral changes. Examples of this + /// include: + /// + /// - If a field's value performs an action when it is dropped. + /// - If a field's type does not implement an auto trait + /// (e.g. `Send`, `Sync`, `Unpin`). + /// + /// For side-effects from dropping field values, this lint should + /// be allowed on those fields. For side-effects from containing + /// field types, `PhantomData` should be used. pub DEAD_CODE, Warn, "detect unused, unexported items" From 15b71f491f39c0d2e3b733c13328f3b513900309 Mon Sep 17 00:00:00 2001 From: ltdk Date: Sun, 13 Nov 2022 04:09:20 -0500 Subject: [PATCH 10/33] 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 7f427f86bd0db622f896bcdce04ce5f20658c071 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 6 Jan 2024 13:17:51 -0700 Subject: [PATCH 11/33] rustdoc-search: parse and search with ML-style HOF Option::map, for example, looks like this: option, (t -> u) -> option This syntax searches all of the HOFs in Rust: traits Fn, FnOnce, and FnMut, and bare fn primitives. --- .../rustdoc/src/read-documentation/search.md | 8 +- src/librustdoc/html/render/search_index.rs | 40 +- src/librustdoc/html/static/js/search.js | 166 ++++++-- tests/rustdoc-js-std/parser-errors.js | 4 +- tests/rustdoc-js-std/parser-hof.js | 376 ++++++++++++++++++ tests/rustdoc-js/hof.js | 94 +++++ tests/rustdoc-js/hof.rs | 12 + 7 files changed, 650 insertions(+), 50 deletions(-) create mode 100644 tests/rustdoc-js-std/parser-hof.js create mode 100644 tests/rustdoc-js/hof.js create mode 100644 tests/rustdoc-js/hof.rs diff --git a/src/doc/rustdoc/src/read-documentation/search.md b/src/doc/rustdoc/src/read-documentation/search.md index b5f4060f05905..d17e41bcde7de 100644 --- a/src/doc/rustdoc/src/read-documentation/search.md +++ b/src/doc/rustdoc/src/read-documentation/search.md @@ -63,11 +63,12 @@ Before describing the syntax in more detail, here's a few sample searches of the standard library and functions that are included in the results list: | Query | Results | -|-------|--------| +|-------|---------| | [`usize -> vec`][] | `slice::repeat` and `Vec::with_capacity` | | [`vec, vec -> bool`][] | `Vec::eq` | | [`option, fnonce -> option`][] | `Option::map` and `Option::and_then` | -| [`option, fnonce -> option`][] | `Option::filter` and `Option::inspect` | +| [`option, (fnonce (T) -> bool) -> option`][optionfilter] | `Option::filter` | +| [`option, (T -> bool) -> option`][optionfilter2] | `Option::filter` | | [`option -> default`][] | `Option::unwrap_or_default` | | [`stdout, [u8]`][stdoutu8] | `Stdout::write` | | [`any -> !`][] | `panic::panic_any` | @@ -77,7 +78,8 @@ the standard library and functions that are included in the results list: [`usize -> vec`]: ../../std/vec/struct.Vec.html?search=usize%20-%3E%20vec&filter-crate=std [`vec, vec -> bool`]: ../../std/vec/struct.Vec.html?search=vec,%20vec%20-%3E%20bool&filter-crate=std [`option, fnonce -> option`]: ../../std/vec/struct.Vec.html?search=option%2C%20fnonce%20->%20option&filter-crate=std -[`option, fnonce -> option`]: ../../std/vec/struct.Vec.html?search=option%2C%20fnonce%20->%20option&filter-crate=std +[optionfilter]: ../../std/vec/struct.Vec.html?search=option%2C+(fnonce+(T)+->+bool)+->+option&filter-crate=std +[optionfilter2]: ../../std/vec/struct.Vec.html?search=option%2C+(T+->+bool)+->+option&filter-crate=std [`option -> default`]: ../../std/vec/struct.Vec.html?search=option%20-%3E%20default&filter-crate=std [`any -> !`]: ../../std/vec/struct.Vec.html?search=any%20-%3E%20!&filter-crate=std [stdoutu8]: ../../std/vec/struct.Vec.html?search=stdout%2C%20[u8]&filter-crate=std diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index cb059082f85ba..f153a90832910 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -4,6 +4,7 @@ use std::collections::{BTreeMap, VecDeque}; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::DefId; +use rustc_span::sym; use rustc_span::symbol::Symbol; use serde::ser::{Serialize, SerializeSeq, SerializeStruct, Serializer}; use thin_vec::ThinVec; @@ -566,6 +567,7 @@ fn get_index_type_id( // The type parameters are converted to generics in `simplify_fn_type` clean::Slice(_) => Some(RenderTypeId::Primitive(clean::PrimitiveType::Slice)), clean::Array(_, _) => Some(RenderTypeId::Primitive(clean::PrimitiveType::Array)), + clean::BareFunction(_) => Some(RenderTypeId::Primitive(clean::PrimitiveType::Fn)), clean::Tuple(ref n) if n.is_empty() => { Some(RenderTypeId::Primitive(clean::PrimitiveType::Unit)) } @@ -584,7 +586,7 @@ fn get_index_type_id( } } // Not supported yet - clean::BareFunction(_) | clean::Generic(_) | clean::ImplTrait(_) | clean::Infer => None, + clean::Generic(_) | clean::ImplTrait(_) | clean::Infer => None, } } @@ -785,6 +787,42 @@ fn simplify_fn_type<'tcx, 'a>( ); } res.push(get_index_type(arg, ty_generics, rgen)); + } else if let Type::BareFunction(ref bf) = *arg { + let mut ty_generics = Vec::new(); + for ty in bf.decl.inputs.values.iter().map(|arg| &arg.type_) { + simplify_fn_type( + self_, + generics, + ty, + tcx, + recurse + 1, + &mut ty_generics, + rgen, + is_return, + cache, + ); + } + // The search index, for simplicity's sake, represents fn pointers and closures + // the same way: as a tuple for the parameters, and an associated type for the + // return type. + let mut ty_output = Vec::new(); + simplify_fn_type( + self_, + generics, + &bf.decl.output, + tcx, + recurse + 1, + &mut ty_output, + rgen, + is_return, + cache, + ); + let ty_bindings = vec![(RenderTypeId::AssociatedType(sym::Output), ty_output)]; + res.push(RenderType { + id: get_index_type_id(&arg, rgen), + bindings: Some(ty_bindings), + generics: Some(ty_generics), + }); } else { // This is not a type parameter. So for example if we have `T, U: Option`, and we're // looking at `Option`, we enter this "else" condition, otherwise if it's `T`, we don't. diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 7995a33f09f9b..1e163367b512a 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -272,6 +272,22 @@ function initSearch(rawSearchIndex) { * Special type name IDs for searching by both tuple and unit (`()` syntax). */ let typeNameIdOfTupleOrUnit; + /** + * Special type name IDs for searching `fn`. + */ + let typeNameIdOfFn; + /** + * Special type name IDs for searching `fnmut`. + */ + let typeNameIdOfFnMut; + /** + * Special type name IDs for searching `fnonce`. + */ + let typeNameIdOfFnOnce; + /** + * Special type name IDs for searching higher order functions (`->` syntax). + */ + let typeNameIdOfHof; /** * Add an item to the type Name->ID map, or, if one already exists, use it. @@ -464,6 +480,21 @@ function initSearch(rawSearchIndex) { } } + function makePrimitiveElement(name, extra) { + return Object.assign({ + name, + id: null, + fullPath: [name], + pathWithoutLast: [], + pathLast: name, + normalizedPathLast: name, + generics: [], + bindings: new Map(), + typeFilter: "primitive", + bindingName: null, + }, extra); + } + /** * @param {ParsedQuery} query * @param {ParserState} parserState @@ -501,18 +532,7 @@ function initSearch(rawSearchIndex) { } const bindingName = parserState.isInBinding; parserState.isInBinding = null; - return { - name: "never", - id: null, - fullPath: ["never"], - pathWithoutLast: [], - pathLast: "never", - normalizedPathLast: "never", - generics: [], - bindings: new Map(), - typeFilter: "primitive", - bindingName, - }; + return makePrimitiveElement("never", { bindingName }); } const quadcolon = /::\s*::/.exec(path); if (path.startsWith("::")) { @@ -671,28 +691,19 @@ function initSearch(rawSearchIndex) { let start = parserState.pos; let end; if ("[(".indexOf(parserState.userQuery[parserState.pos]) !== -1) { -let endChar = ")"; -let name = "()"; -let friendlyName = "tuple"; - -if (parserState.userQuery[parserState.pos] === "[") { - endChar = "]"; - name = "[]"; - friendlyName = "slice"; -} + let endChar = ")"; + let name = "()"; + let friendlyName = "tuple"; + + if (parserState.userQuery[parserState.pos] === "[") { + endChar = "]"; + name = "[]"; + friendlyName = "slice"; + } parserState.pos += 1; const { foundSeparator } = getItemsBefore(query, parserState, generics, endChar); const typeFilter = parserState.typeFilter; - const isInBinding = parserState.isInBinding; - if (typeFilter !== null && typeFilter !== "primitive") { - throw [ - "Invalid search type: primitive ", - name, - " and ", - typeFilter, - " both specified", - ]; - } + const bindingName = parserState.isInBinding; parserState.typeFilter = null; parserState.isInBinding = null; for (const gen of generics) { @@ -702,23 +713,26 @@ if (parserState.userQuery[parserState.pos] === "[") { } if (name === "()" && !foundSeparator && generics.length === 1 && typeFilter === null) { elems.push(generics[0]); + } else if (name === "()" && generics.length === 1 && generics[0].name === "->") { + // `primitive:(a -> b)` parser to `primitive:"->"` + // not `primitive:"()"<"->">` + generics[0].typeFilter = typeFilter; + elems.push(generics[0]); } else { + if (typeFilter !== null && typeFilter !== "primitive") { + throw [ + "Invalid search type: primitive ", + name, + " and ", + typeFilter, + " both specified", + ]; + } parserState.totalElems += 1; if (isInGenerics) { parserState.genericsElems += 1; } - elems.push({ - name: name, - id: null, - fullPath: [name], - pathWithoutLast: [], - pathLast: name, - normalizedPathLast: name, - generics, - bindings: new Map(), - typeFilter: "primitive", - bindingName: isInBinding, - }); + elems.push(makePrimitiveElement(name, { bindingName, generics })); } } else { const isStringElem = parserState.userQuery[start] === "\""; @@ -805,6 +819,19 @@ if (parserState.userQuery[parserState.pos] === "[") { const oldIsInBinding = parserState.isInBinding; parserState.isInBinding = null; + // ML-style Higher Order Function notation + // + // a way to search for any closure or fn pointer regardless of + // which closure trait is used + // + // Looks like this: + // + // `option, (t -> u) -> option` + // ^^^^^^ + // + // The Rust-style closure notation is implemented in getNextElem + let hofParameters = null; + let extra = ""; if (endChar === ">") { extra = "<"; @@ -825,6 +852,21 @@ if (parserState.userQuery[parserState.pos] === "[") { throw ["Unexpected ", endChar, " after ", "="]; } break; + } else if (endChar !== "" && isReturnArrow(parserState)) { + // ML-style HOF notation only works when delimited in something, + // otherwise a function arrow starts the return type of the top + if (parserState.isInBinding) { + throw ["Unexpected ", "->", " after ", "="]; + } + hofParameters = [...elems]; + elems.length = 0; + parserState.pos += 2; + foundStopChar = true; + foundSeparator = false; + continue; + } else if (c === " ") { + parserState.pos += 1; + continue; } else if (isSeparatorCharacter(c)) { parserState.pos += 1; foundStopChar = true; @@ -904,6 +946,27 @@ if (parserState.userQuery[parserState.pos] === "[") { // in any case. parserState.pos += 1; + if (hofParameters) { + // Commas in a HOF don't cause wrapping parens to become a tuple. + // If you want a one-tuple with a HOF in it, write `((a -> b),)`. + foundSeparator = false; + // HOFs can't have directly nested bindings. + if ([...elems, ...hofParameters].some(x => x.bindingName) || parserState.isInBinding) { + throw ["Unexpected ", "=", " within ", "->"]; + } + // HOFs are represented the same way closures are. + // The arguments are wrapped in a tuple, and the output + // is a binding, even though the compiler doesn't technically + // represent fn pointers that way. + const hofElem = makePrimitiveElement("->", { + generics: hofParameters, + bindings: new Map([["output", [...elems]]]), + typeFilter: null, + }); + elems.length = 0; + elems[0] = hofElem; + } + parserState.typeFilter = oldTypeFilter; parserState.isInBinding = oldIsInBinding; @@ -1635,6 +1698,12 @@ if (parserState.userQuery[parserState.pos] === "[") { ) { // () matches primitive:tuple or primitive:unit // if it matches, then we're fine, and this is an appropriate match candidate + } else if (queryElem.id === typeNameIdOfHof && + (fnType.id === typeNameIdOfFn || fnType.id === typeNameIdOfFnMut || + fnType.id === typeNameIdOfFnOnce) + ) { + // -> matches fn, fnonce, and fnmut + // if it matches, then we're fine, and this is an appropriate match candidate } else if (fnType.id !== queryElem.id || queryElem.id === null) { return false; } @@ -1829,6 +1898,7 @@ if (parserState.userQuery[parserState.pos] === "[") { typePassesFilter(elem.typeFilter, row.ty) && elem.generics.length === 0 && // special case elem.id !== typeNameIdOfArrayOrSlice && elem.id !== typeNameIdOfTupleOrUnit + && elem.id !== typeNameIdOfHof ) { return row.id === elem.id || checkIfInList( row.generics, @@ -2991,7 +3061,7 @@ ${item.displayPath}${name}\ */ function buildFunctionTypeFingerprint(type, output, fps) { let input = type.id; - // All forms of `[]`/`()` get collapsed down to one thing in the bloom filter. + // All forms of `[]`/`()`/`->` get collapsed down to one thing in the bloom filter. // Differentiating between arrays and slices, if the user asks for it, is // still done in the matching algorithm. if (input === typeNameIdOfArray || input === typeNameIdOfSlice) { @@ -3000,6 +3070,10 @@ ${item.displayPath}${name}\ if (input === typeNameIdOfTuple || input === typeNameIdOfUnit) { input = typeNameIdOfTupleOrUnit; } + if (input === typeNameIdOfFn || input === typeNameIdOfFnMut || + input === typeNameIdOfFnOnce) { + input = typeNameIdOfHof; + } // http://burtleburtle.net/bob/hash/integer.html // ~~ is toInt32. It's used before adding, so // the number stays in safe integer range. @@ -3103,6 +3177,10 @@ ${item.displayPath}${name}\ typeNameIdOfUnit = buildTypeMapIndex("unit"); typeNameIdOfArrayOrSlice = buildTypeMapIndex("[]"); typeNameIdOfTupleOrUnit = buildTypeMapIndex("()"); + typeNameIdOfFn = buildTypeMapIndex("fn"); + typeNameIdOfFnMut = buildTypeMapIndex("fnmut"); + typeNameIdOfFnOnce = buildTypeMapIndex("fnonce"); + typeNameIdOfHof = buildTypeMapIndex("->"); // Function type fingerprints are 128-bit bloom filters that are used to // estimate the distance between function and query. diff --git a/tests/rustdoc-js-std/parser-errors.js b/tests/rustdoc-js-std/parser-errors.js index 16d171260dadb..8efb81841d47f 100644 --- a/tests/rustdoc-js-std/parser-errors.js +++ b/tests/rustdoc-js-std/parser-errors.js @@ -114,7 +114,7 @@ const PARSED = [ original: "(p -> p", returned: [], userQuery: "(p -> p", - error: "Unexpected `-` after `(`", + error: "Unclosed `(`", }, { query: "::a::b", @@ -330,7 +330,7 @@ const PARSED = [ original: 'a<->', returned: [], userQuery: 'a<->', - error: 'Unexpected `-` after `<`', + error: 'Unclosed `<`', }, { query: "a:", diff --git a/tests/rustdoc-js-std/parser-hof.js b/tests/rustdoc-js-std/parser-hof.js new file mode 100644 index 0000000000000..331c516e047a9 --- /dev/null +++ b/tests/rustdoc-js-std/parser-hof.js @@ -0,0 +1,376 @@ +const PARSED = [ + { + query: "(-> F

)", + elems: [{ + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [], + bindings: [ + [ + "output", + [{ + name: "f", + fullPath: ["f"], + pathWithoutLast: [], + pathLast: "f", + generics: [ + { + name: "p", + fullPath: ["p"], + pathWithoutLast: [], + pathLast: "p", + generics: [], + }, + ], + typeFilter: -1, + }], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "(-> F

)", + returned: [], + userQuery: "(-> f

)", + error: null, + }, + { + query: "(-> P)", + elems: [{ + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [], + bindings: [ + [ + "output", + [{ + name: "p", + fullPath: ["p"], + pathWithoutLast: [], + pathLast: "p", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "(-> P)", + returned: [], + userQuery: "(-> p)", + error: null, + }, + { + query: "(->,a)", + elems: [{ + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [], + bindings: [ + [ + "output", + [{ + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "(->,a)", + returned: [], + userQuery: "(->,a)", + error: null, + }, + { + query: "(F

->)", + elems: [{ + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [{ + name: "f", + fullPath: ["f"], + pathWithoutLast: [], + pathLast: "f", + generics: [ + { + name: "p", + fullPath: ["p"], + pathWithoutLast: [], + pathLast: "p", + generics: [], + }, + ], + typeFilter: -1, + }], + bindings: [ + [ + "output", + [], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "(F

->)", + returned: [], + userQuery: "(f

->)", + error: null, + }, + { + query: "(P ->)", + elems: [{ + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [{ + name: "p", + fullPath: ["p"], + pathWithoutLast: [], + pathLast: "p", + generics: [], + typeFilter: -1, + }], + bindings: [ + [ + "output", + [], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "(P ->)", + returned: [], + userQuery: "(p ->)", + error: null, + }, + { + query: "(,a->)", + elems: [{ + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [{ + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: -1, + }], + bindings: [ + [ + "output", + [], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "(,a->)", + returned: [], + userQuery: "(,a->)", + error: null, + }, + { + query: "(aaaaa->a)", + elems: [{ + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [{ + name: "aaaaa", + fullPath: ["aaaaa"], + pathWithoutLast: [], + pathLast: "aaaaa", + generics: [], + typeFilter: -1, + }], + bindings: [ + [ + "output", + [{ + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "(aaaaa->a)", + returned: [], + userQuery: "(aaaaa->a)", + error: null, + }, + { + query: "(aaaaa, b -> a)", + elems: [{ + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [ + { + name: "aaaaa", + fullPath: ["aaaaa"], + pathWithoutLast: [], + pathLast: "aaaaa", + generics: [], + typeFilter: -1, + }, + { + name: "b", + fullPath: ["b"], + pathWithoutLast: [], + pathLast: "b", + generics: [], + typeFilter: -1, + }, + ], + bindings: [ + [ + "output", + [{ + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "(aaaaa, b -> a)", + returned: [], + userQuery: "(aaaaa, b -> a)", + error: null, + }, + { + query: "primitive:(aaaaa, b -> a)", + elems: [{ + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [ + { + name: "aaaaa", + fullPath: ["aaaaa"], + pathWithoutLast: [], + pathLast: "aaaaa", + generics: [], + typeFilter: -1, + }, + { + name: "b", + fullPath: ["b"], + pathWithoutLast: [], + pathLast: "b", + generics: [], + typeFilter: -1, + }, + ], + bindings: [ + [ + "output", + [{ + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: 1, + }], + foundElems: 1, + original: "primitive:(aaaaa, b -> a)", + returned: [], + userQuery: "primitive:(aaaaa, b -> a)", + error: null, + }, + { + query: "x, trait:(aaaaa, b -> a)", + elems: [ + { + name: "x", + fullPath: ["x"], + pathWithoutLast: [], + pathLast: "x", + generics: [], + typeFilter: -1, + }, + { + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [ + { + name: "aaaaa", + fullPath: ["aaaaa"], + pathWithoutLast: [], + pathLast: "aaaaa", + generics: [], + typeFilter: -1, + }, + { + name: "b", + fullPath: ["b"], + pathWithoutLast: [], + pathLast: "b", + generics: [], + typeFilter: -1, + }, + ], + bindings: [ + [ + "output", + [{ + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: 10, + } + ], + foundElems: 2, + original: "x, trait:(aaaaa, b -> a)", + returned: [], + userQuery: "x, trait:(aaaaa, b -> a)", + error: null, + }, +]; diff --git a/tests/rustdoc-js/hof.js b/tests/rustdoc-js/hof.js new file mode 100644 index 0000000000000..1f5d2bfb6666c --- /dev/null +++ b/tests/rustdoc-js/hof.js @@ -0,0 +1,94 @@ +// exact-check + +const EXPECTED = [ + // ML-style higher-order function notation + { + 'query': 'bool, (u32 -> !) -> ()', + 'others': [ + {"path": "hof", "name": "fn_ptr"}, + ], + }, + { + 'query': 'u8, (u32 -> !) -> ()', + 'others': [ + {"path": "hof", "name": "fn_once"}, + ], + }, + { + 'query': 'i8, (u32 -> !) -> ()', + 'others': [ + {"path": "hof", "name": "fn_mut"}, + ], + }, + { + 'query': 'char, (u32 -> !) -> ()', + 'others': [ + {"path": "hof", "name": "fn_"}, + ], + }, + { + 'query': '(first -> !) -> ()', + 'others': [ + {"path": "hof", "name": "fn_ptr"}, + ], + }, + { + 'query': '(second -> !) -> ()', + 'others': [ + {"path": "hof", "name": "fn_once"}, + ], + }, + { + 'query': '(third -> !) -> ()', + 'others': [ + {"path": "hof", "name": "fn_mut"}, + ], + }, + { + 'query': '(u32 -> !) -> ()', + 'others': [ + {"path": "hof", "name": "fn_"}, + {"path": "hof", "name": "fn_ptr"}, + {"path": "hof", "name": "fn_mut"}, + {"path": "hof", "name": "fn_once"}, + ], + }, + { + 'query': 'u32 -> !', + // not a HOF query + 'others': [], + }, + { + 'query': '(str, str -> i8) -> ()', + 'others': [ + {"path": "hof", "name": "multiple"}, + ], + }, + { + 'query': '(str ->) -> ()', + 'others': [ + {"path": "hof", "name": "multiple"}, + ], + }, + { + 'query': '(-> i8) -> ()', + 'others': [ + {"path": "hof", "name": "multiple"}, + ], + }, + { + 'query': '(str -> str) -> ()', + // params and return are not the same + 'others': [], + }, + { + 'query': '(i8 ->) -> ()', + // params and return are not the same + 'others': [], + }, + { + 'query': '(-> str) -> ()', + // params and return are not the same + 'others': [], + }, +]; diff --git a/tests/rustdoc-js/hof.rs b/tests/rustdoc-js/hof.rs new file mode 100644 index 0000000000000..4d2c6e331cac0 --- /dev/null +++ b/tests/rustdoc-js/hof.rs @@ -0,0 +1,12 @@ +#![feature(never_type)] + +pub struct First(T); +pub struct Second(T); +pub struct Third(T); + +pub fn fn_ptr(_: fn (First) -> !, _: bool) {} +pub fn fn_once(_: impl FnOnce (Second) -> !, _: u8) {} +pub fn fn_mut(_: impl FnMut (Third) -> !, _: i8) {} +pub fn fn_(_: impl Fn (u32) -> !, _: char) {} + +pub fn multiple(_: impl Fn(&'static str, &'static str) -> i8) {} From d38527eb82fd10ee5578b541b06904611f6f8427 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 6 Jan 2024 13:45:52 -0700 Subject: [PATCH 12/33] rustdoc: clean up search.js by removing empty sort case It's going to be a no-op on the empty list anyway (we have plenty of test cases that return nothing) so why send extra code? --- src/librustdoc/html/static/js/search.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 1e163367b512a..f8d1714e86603 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -1321,11 +1321,6 @@ function initSearch(rawSearchIndex) { * @returns {[ResultObject]} */ function sortResults(results, isType, preferredCrate) { - // if there are no results then return to default and fail - if (results.size === 0) { - return []; - } - const userQuery = parsedQuery.userQuery; const result_list = []; for (const result of results.values()) { From 23e931fd076cc1d02a34b3f9bd9a64f92c8f4289 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 6 Jan 2024 14:07:12 -0700 Subject: [PATCH 13/33] rustdoc: use `const` for the special type name ids Initialize them before the search index is loaded. --- src/librustdoc/html/static/js/search.js | 36 ++++++++----------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index f8d1714e86603..6c285ba6f5102 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -245,49 +245,49 @@ function initSearch(rawSearchIndex) { * * @type {Map} */ - let typeNameIdMap; + const typeNameIdMap = new Map(); const ALIASES = new Map(); /** * Special type name IDs for searching by array. */ - let typeNameIdOfArray; + const typeNameIdOfArray = buildTypeMapIndex("array"); /** * Special type name IDs for searching by slice. */ - let typeNameIdOfSlice; + const typeNameIdOfSlice = buildTypeMapIndex("slice"); /** * Special type name IDs for searching by both array and slice (`[]` syntax). */ - let typeNameIdOfArrayOrSlice; + const typeNameIdOfArrayOrSlice = buildTypeMapIndex("[]"); /** * Special type name IDs for searching by tuple. */ - let typeNameIdOfTuple; + const typeNameIdOfTuple = buildTypeMapIndex("tuple"); /** * Special type name IDs for searching by unit. */ - let typeNameIdOfUnit; + const typeNameIdOfUnit = buildTypeMapIndex("unit"); /** * Special type name IDs for searching by both tuple and unit (`()` syntax). */ - let typeNameIdOfTupleOrUnit; + const typeNameIdOfTupleOrUnit = buildTypeMapIndex("()"); /** * Special type name IDs for searching `fn`. */ - let typeNameIdOfFn; + const typeNameIdOfFn = buildTypeMapIndex("fn"); /** * Special type name IDs for searching `fnmut`. */ - let typeNameIdOfFnMut; + const typeNameIdOfFnMut = buildTypeMapIndex("fnmut"); /** * Special type name IDs for searching `fnonce`. */ - let typeNameIdOfFnOnce; + const typeNameIdOfFnOnce = buildTypeMapIndex("fnonce"); /** * Special type name IDs for searching higher order functions (`->` syntax). */ - let typeNameIdOfHof; + const typeNameIdOfHof = buildTypeMapIndex("->"); /** * Add an item to the type Name->ID map, or, if one already exists, use it. @@ -3159,24 +3159,10 @@ ${item.displayPath}${name}\ */ function buildIndex(rawSearchIndex) { searchIndex = []; - typeNameIdMap = new Map(); const charA = "A".charCodeAt(0); let currentIndex = 0; let id = 0; - // Initialize type map indexes for primitive list types - // that can be searched using `[]` syntax. - typeNameIdOfArray = buildTypeMapIndex("array"); - typeNameIdOfSlice = buildTypeMapIndex("slice"); - typeNameIdOfTuple = buildTypeMapIndex("tuple"); - typeNameIdOfUnit = buildTypeMapIndex("unit"); - typeNameIdOfArrayOrSlice = buildTypeMapIndex("[]"); - typeNameIdOfTupleOrUnit = buildTypeMapIndex("()"); - typeNameIdOfFn = buildTypeMapIndex("fn"); - typeNameIdOfFnMut = buildTypeMapIndex("fnmut"); - typeNameIdOfFnOnce = buildTypeMapIndex("fnonce"); - typeNameIdOfHof = buildTypeMapIndex("->"); - // Function type fingerprints are 128-bit bloom filters that are used to // estimate the distance between function and query. // This loop counts the number of items to allocate a fingerprint for. From 7b926555b71032889ea5079aa642885e63fe51c2 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 6 Jan 2024 16:01:10 -0700 Subject: [PATCH 14/33] rustdoc-search: add search query syntax `Fn(T) -> U` This is implemented, in addition to the ML-style one, because Rust does it. If we don't, we'll never hear the end of it. This commit also refactors some duplicate parts of the parser into a dedicated function. --- .../rustdoc/src/read-documentation/search.md | 46 ++- src/librustdoc/html/static/js/search.js | 113 +++--- tests/rustdoc-js-std/parser-errors.js | 15 +- tests/rustdoc-js-std/parser-hof.js | 336 ++++++++++++++++++ tests/rustdoc-js-std/parser-weird-queries.js | 9 - tests/rustdoc-js/hof.js | 92 ++++- 6 files changed, 530 insertions(+), 81 deletions(-) diff --git a/src/doc/rustdoc/src/read-documentation/search.md b/src/doc/rustdoc/src/read-documentation/search.md index d17e41bcde7de..e2def14b357ea 100644 --- a/src/doc/rustdoc/src/read-documentation/search.md +++ b/src/doc/rustdoc/src/read-documentation/search.md @@ -153,16 +153,26 @@ will match these queries: But it *does not* match `Result` or `Result>`. +To search for a function that accepts a function as a parameter, +like `Iterator::all`, wrap the nested signature in parenthesis, +as in [`Iterator, (T -> bool) -> bool`][iterator-all]. +You can also search for a specific closure trait, +such as `Iterator, (FnMut(T) -> bool) -> bool`, +but you need to know which one you want. + +[iterator-all]: ../../std/vec/struct.Vec.html?search=Iterator%2C+(T+->+bool)+->+bool&filter-crate=std + ### Primitives with Special Syntax -| Shorthand | Explicit names | -| --------- | ------------------------------------------------ | -| `[]` | `primitive:slice` and/or `primitive:array` | -| `[T]` | `primitive:slice` and/or `primitive:array` | -| `()` | `primitive:unit` and/or `primitive:tuple` | -| `(T)` | `T` | -| `(T,)` | `primitive:tuple` | -| `!` | `primitive:never` | +| Shorthand | Explicit names | +| ---------------- | ------------------------------------------------- | +| `[]` | `primitive:slice` and/or `primitive:array` | +| `[T]` | `primitive:slice` and/or `primitive:array` | +| `()` | `primitive:unit` and/or `primitive:tuple` | +| `(T)` | `T` | +| `(T,)` | `primitive:tuple` | +| `!` | `primitive:never` | +| `(T, U -> V, W)` | `fn(T, U) -> (V, W)`, `Fn`, `FnMut`, and `FnOnce` | When searching for `[]`, Rustdoc will return search results with either slices or arrays. If you know which one you want, you can force it to return results @@ -182,6 +192,10 @@ results for types that match tuples, even though it also matches the type on its own. That is, `(u32)` matches `(u32,)` for the exact same reason that it also matches `Result`. +The `->` operator has lower precedence than comma. If it's not wrapped +in brackets, it delimits the return value for the function being searched for. +To search for functions that take functions as parameters, use parenthesis. + ### Limitations and quirks of type-based search Type-based search is still a buggy, experimental, work-in-progress feature. @@ -220,9 +234,6 @@ Most of these limitations should be addressed in future version of Rustdoc. * Searching for lifetimes is not supported. - * It's impossible to search for closures based on their parameters or - return values. - * It's impossible to search based on the length of an array. ## Item filtering @@ -239,19 +250,21 @@ Item filters can be used in both name-based and type signature-based searches. ```text ident = *(ALPHA / DIGIT / "_") -path = ident *(DOUBLE-COLON ident) [!] +path = ident *(DOUBLE-COLON ident) [BANG] slice-like = OPEN-SQUARE-BRACKET [ nonempty-arg-list ] CLOSE-SQUARE-BRACKET tuple-like = OPEN-PAREN [ nonempty-arg-list ] CLOSE-PAREN -arg = [type-filter *WS COLON *WS] (path [generics] / slice-like / tuple-like / [!]) +arg = [type-filter *WS COLON *WS] (path [generics] / slice-like / tuple-like) type-sep = COMMA/WS *(COMMA/WS) -nonempty-arg-list = *(type-sep) arg *(type-sep arg) *(type-sep) +nonempty-arg-list = *(type-sep) arg *(type-sep arg) *(type-sep) [ return-args ] generic-arg-list = *(type-sep) arg [ EQUAL arg ] *(type-sep arg [ EQUAL arg ]) *(type-sep) -generics = OPEN-ANGLE-BRACKET [ generic-arg-list ] *(type-sep) +normal-generics = OPEN-ANGLE-BRACKET [ generic-arg-list ] *(type-sep) CLOSE-ANGLE-BRACKET +fn-like-generics = OPEN-PAREN [ nonempty-arg-list ] CLOSE-PAREN [ RETURN-ARROW arg ] +generics = normal-generics / fn-like-generics return-args = RETURN-ARROW *(type-sep) nonempty-arg-list exact-search = [type-filter *WS COLON] [ RETURN-ARROW ] *WS QUOTE ident QUOTE [ generics ] -type-search = [ nonempty-arg-list ] [ return-args ] +type-search = [ nonempty-arg-list ] query = *WS (exact-search / type-search) *WS @@ -296,6 +309,7 @@ QUOTE = %x22 COMMA = "," RETURN-ARROW = "->" EQUAL = "=" +BANG = "!" ALPHA = %x41-5A / %x61-7A ; A-Z / a-z DIGIT = %x30-39 diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 6c285ba6f5102..73ac5608479b1 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -1,3 +1,4 @@ +// ignore-tidy-filelength /* global addClass, getNakedUrl, getSettingValue */ /* global onEachLazy, removeClass, searchState, browserSupportsHistoryApi, exports */ @@ -578,7 +579,10 @@ function initSearch(rawSearchIndex) { // Syntactically, bindings are parsed as generics, // but the query engine treats them differently. if (gen.bindingName !== null) { - bindings.set(gen.bindingName.name, [gen, ...gen.bindingName.generics]); + if (gen.name !== null) { + gen.bindingName.generics.unshift(gen); + } + bindings.set(gen.bindingName.name, gen.bindingName.generics); return false; } return true; @@ -678,6 +682,38 @@ function initSearch(rawSearchIndex) { return end; } + function getFilteredNextElem(query, parserState, elems, isInGenerics) { + const start = parserState.pos; + if (parserState.userQuery[parserState.pos] === ":" && !isPathStart(parserState)) { + throw ["Expected type filter before ", ":"]; + } + getNextElem(query, parserState, elems, isInGenerics); + if (parserState.userQuery[parserState.pos] === ":" && !isPathStart(parserState)) { + if (parserState.typeFilter !== null) { + throw [ + "Unexpected ", + ":", + " (expected path after type filter ", + parserState.typeFilter + ":", + ")", + ]; + } + if (elems.length === 0) { + throw ["Expected type filter before ", ":"]; + } else if (query.literalSearch) { + throw ["Cannot use quotes on type filter"]; + } + // The type filter doesn't count as an element since it's a modifier. + const typeFilterElem = elems.pop(); + checkExtraTypeFilterCharacters(start, parserState); + parserState.typeFilter = typeFilterElem.name; + parserState.pos += 1; + parserState.totalElems -= 1; + query.literalSearch = false; + getNextElem(query, parserState, elems, isInGenerics); + } + } + /** * @param {ParsedQuery} query * @param {ParserState} parserState @@ -752,6 +788,32 @@ function initSearch(rawSearchIndex) { } parserState.pos += 1; getItemsBefore(query, parserState, generics, ">"); + } else if (parserState.pos < parserState.length && + parserState.userQuery[parserState.pos] === "(" + ) { + if (start >= end) { + throw ["Found generics without a path"]; + } + if (parserState.isInBinding) { + throw ["Unexpected ", "(", " after ", "="]; + } + parserState.pos += 1; + const typeFilter = parserState.typeFilter; + parserState.typeFilter = null; + getItemsBefore(query, parserState, generics, ")"); + skipWhitespace(parserState); + if (isReturnArrow(parserState)) { + parserState.pos += 2; + skipWhitespace(parserState); + getFilteredNextElem(query, parserState, generics, isInGenerics); + generics[generics.length - 1].bindingName = makePrimitiveElement("output"); + } else { + generics.push(makePrimitiveElement(null, { + bindingName: makePrimitiveElement("output"), + typeFilter: null, + })); + } + parserState.typeFilter = typeFilter; } if (isStringElem) { skipWhitespace(parserState); @@ -811,7 +873,6 @@ function initSearch(rawSearchIndex) { function getItemsBefore(query, parserState, elems, endChar) { let foundStopChar = true; let foundSeparator = false; - let start = parserState.pos; // If this is a generic, keep the outer item's type filter around. const oldTypeFilter = parserState.typeFilter; @@ -874,24 +935,6 @@ function initSearch(rawSearchIndex) { continue; } else if (c === ":" && isPathStart(parserState)) { throw ["Unexpected ", "::", ": paths cannot start with ", "::"]; - } else if (c === ":") { - if (parserState.typeFilter !== null) { - throw ["Unexpected ", ":"]; - } - if (elems.length === 0) { - throw ["Expected type filter before ", ":"]; - } else if (query.literalSearch) { - throw ["Cannot use quotes on type filter"]; - } - // The type filter doesn't count as an element since it's a modifier. - const typeFilterElem = elems.pop(); - checkExtraTypeFilterCharacters(start, parserState); - parserState.typeFilter = typeFilterElem.name; - parserState.pos += 1; - parserState.totalElems -= 1; - query.literalSearch = false; - foundStopChar = true; - continue; } else if (isEndCharacter(c)) { throw ["Unexpected ", c, " after ", extra]; } @@ -926,8 +969,7 @@ function initSearch(rawSearchIndex) { ]; } const posBefore = parserState.pos; - start = parserState.pos; - getNextElem(query, parserState, elems, endChar !== ""); + getFilteredNextElem(query, parserState, elems, endChar !== ""); if (endChar !== "" && parserState.pos >= parserState.length) { throw ["Unclosed ", extra]; } @@ -1004,7 +1046,6 @@ function initSearch(rawSearchIndex) { */ function parseInput(query, parserState) { let foundStopChar = true; - let start = parserState.pos; while (parserState.pos < parserState.length) { const c = parserState.userQuery[parserState.pos]; @@ -1022,29 +1063,6 @@ function initSearch(rawSearchIndex) { throw ["Unexpected ", c, " after ", parserState.userQuery[parserState.pos - 1]]; } throw ["Unexpected ", c]; - } else if (c === ":" && !isPathStart(parserState)) { - if (parserState.typeFilter !== null) { - throw [ - "Unexpected ", - ":", - " (expected path after type filter ", - parserState.typeFilter + ":", - ")", - ]; - } else if (query.elems.length === 0) { - throw ["Expected type filter before ", ":"]; - } else if (query.literalSearch) { - throw ["Cannot use quotes on type filter"]; - } - // The type filter doesn't count as an element since it's a modifier. - const typeFilterElem = query.elems.pop(); - checkExtraTypeFilterCharacters(start, parserState); - parserState.typeFilter = typeFilterElem.name; - parserState.pos += 1; - parserState.totalElems -= 1; - query.literalSearch = false; - foundStopChar = true; - continue; } else if (c === " ") { skipWhitespace(parserState); continue; @@ -1080,8 +1098,7 @@ function initSearch(rawSearchIndex) { ]; } const before = query.elems.length; - start = parserState.pos; - getNextElem(query, parserState, query.elems, false); + getFilteredNextElem(query, parserState, query.elems, false); if (query.elems.length === before) { // Nothing was added, weird... Let's increase the position to not remain stuck. parserState.pos += 1; diff --git a/tests/rustdoc-js-std/parser-errors.js b/tests/rustdoc-js-std/parser-errors.js index 8efb81841d47f..ffd169812b631 100644 --- a/tests/rustdoc-js-std/parser-errors.js +++ b/tests/rustdoc-js-std/parser-errors.js @@ -195,7 +195,7 @@ const PARSED = [ original: "a (b:", returned: [], userQuery: "a (b:", - error: "Expected `,`, `:` or `->`, found `(`", + error: "Unclosed `(`", }, { query: "_:", @@ -357,7 +357,16 @@ const PARSED = [ original: "a,:", returned: [], userQuery: "a,:", - error: 'Unexpected `,` in type filter (before `:`)', + error: 'Expected type filter before `:`', + }, + { + query: "a!:", + elems: [], + foundElems: 0, + original: "a!:", + returned: [], + userQuery: "a!:", + error: 'Unexpected `!` in type filter (before `:`)', }, { query: " a<> :", @@ -366,7 +375,7 @@ const PARSED = [ original: "a<> :", returned: [], userQuery: "a<> :", - error: 'Unexpected `<` in type filter (before `:`)', + error: 'Expected `,`, `:` or `->` after `>`, found `:`', }, { query: "mod : :", diff --git a/tests/rustdoc-js-std/parser-hof.js b/tests/rustdoc-js-std/parser-hof.js index 331c516e047a9..0b99c45b7a922 100644 --- a/tests/rustdoc-js-std/parser-hof.js +++ b/tests/rustdoc-js-std/parser-hof.js @@ -1,4 +1,5 @@ const PARSED = [ + // ML-style HOF { query: "(-> F

)", elems: [{ @@ -373,4 +374,339 @@ const PARSED = [ userQuery: "x, trait:(aaaaa, b -> a)", error: null, }, + // Rust-style HOF + { + query: "Fn () -> F

", + elems: [{ + name: "fn", + fullPath: ["fn"], + pathWithoutLast: [], + pathLast: "fn", + generics: [], + bindings: [ + [ + "output", + [{ + name: "f", + fullPath: ["f"], + pathWithoutLast: [], + pathLast: "f", + generics: [ + { + name: "p", + fullPath: ["p"], + pathWithoutLast: [], + pathLast: "p", + generics: [], + }, + ], + typeFilter: -1, + }], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "Fn () -> F

", + returned: [], + userQuery: "fn () -> f

", + error: null, + }, + { + query: "FnMut() -> P", + elems: [{ + name: "fnmut", + fullPath: ["fnmut"], + pathWithoutLast: [], + pathLast: "fnmut", + generics: [], + bindings: [ + [ + "output", + [{ + name: "p", + fullPath: ["p"], + pathWithoutLast: [], + pathLast: "p", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "FnMut() -> P", + returned: [], + userQuery: "fnmut() -> p", + error: null, + }, + { + query: "(FnMut() -> P)", + elems: [{ + name: "fnmut", + fullPath: ["fnmut"], + pathWithoutLast: [], + pathLast: "fnmut", + generics: [], + bindings: [ + [ + "output", + [{ + name: "p", + fullPath: ["p"], + pathWithoutLast: [], + pathLast: "p", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "(FnMut() -> P)", + returned: [], + userQuery: "(fnmut() -> p)", + error: null, + }, + { + query: "Fn(F

)", + elems: [{ + name: "fn", + fullPath: ["fn"], + pathWithoutLast: [], + pathLast: "fn", + generics: [{ + name: "f", + fullPath: ["f"], + pathWithoutLast: [], + pathLast: "f", + generics: [ + { + name: "p", + fullPath: ["p"], + pathWithoutLast: [], + pathLast: "p", + generics: [], + }, + ], + typeFilter: -1, + }], + bindings: [ + [ + "output", + [], + ], + ], + typeFilter: -1, + }], + foundElems: 1, + original: "Fn(F

)", + returned: [], + userQuery: "fn(f

)", + error: null, + }, + { + query: "primitive:fnonce(aaaaa, b) -> a", + elems: [{ + name: "fnonce", + fullPath: ["fnonce"], + pathWithoutLast: [], + pathLast: "fnonce", + generics: [ + { + name: "aaaaa", + fullPath: ["aaaaa"], + pathWithoutLast: [], + pathLast: "aaaaa", + generics: [], + typeFilter: -1, + }, + { + name: "b", + fullPath: ["b"], + pathWithoutLast: [], + pathLast: "b", + generics: [], + typeFilter: -1, + }, + ], + bindings: [ + [ + "output", + [{ + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: 1, + }], + foundElems: 1, + original: "primitive:fnonce(aaaaa, b) -> a", + returned: [], + userQuery: "primitive:fnonce(aaaaa, b) -> a", + error: null, + }, + { + query: "primitive:fnonce(aaaaa, keyword:b) -> trait:a", + elems: [{ + name: "fnonce", + fullPath: ["fnonce"], + pathWithoutLast: [], + pathLast: "fnonce", + generics: [ + { + name: "aaaaa", + fullPath: ["aaaaa"], + pathWithoutLast: [], + pathLast: "aaaaa", + generics: [], + typeFilter: -1, + }, + { + name: "b", + fullPath: ["b"], + pathWithoutLast: [], + pathLast: "b", + generics: [], + typeFilter: 0, + }, + ], + bindings: [ + [ + "output", + [{ + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: 10, + }], + ], + ], + typeFilter: 1, + }], + foundElems: 1, + original: "primitive:fnonce(aaaaa, keyword:b) -> trait:a", + returned: [], + userQuery: "primitive:fnonce(aaaaa, keyword:b) -> trait:a", + error: null, + }, + { + query: "x, trait:fn(aaaaa, b -> a)", + elems: [ + { + name: "x", + fullPath: ["x"], + pathWithoutLast: [], + pathLast: "x", + generics: [], + typeFilter: -1, + }, + { + name: "fn", + fullPath: ["fn"], + pathWithoutLast: [], + pathLast: "fn", + generics: [ + { + name: "->", + fullPath: ["->"], + pathWithoutLast: [], + pathLast: "->", + generics: [ + { + name: "aaaaa", + fullPath: ["aaaaa"], + pathWithoutLast: [], + pathLast: "aaaaa", + generics: [], + typeFilter: -1, + }, + { + name: "b", + fullPath: ["b"], + pathWithoutLast: [], + pathLast: "b", + generics: [], + typeFilter: -1, + }, + ], + bindings: [ + [ + "output", + [{ + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: -1, + }], + ], + ], + typeFilter: -1, + }, + ], + bindings: [ + [ + "output", + [], + ] + ], + typeFilter: 10, + } + ], + foundElems: 2, + original: "x, trait:fn(aaaaa, b -> a)", + returned: [], + userQuery: "x, trait:fn(aaaaa, b -> a)", + error: null, + }, + { + query: 'a,b(c)', + elems: [ + { + name: "a", + fullPath: ["a"], + pathWithoutLast: [], + pathLast: "a", + generics: [], + typeFilter: -1, + }, + { + name: "b", + fullPath: ["b"], + pathWithoutLast: [], + pathLast: "b", + generics: [{ + name: "c", + fullPath: ["c"], + pathWithoutLast: [], + pathLast: "c", + generics: [], + typeFilter: -1, + }], + bindings: [ + [ + "output", + [], + ] + ], + typeFilter: -1, + } + ], + foundElems: 2, + original: "a,b(c)", + returned: [], + userQuery: "a,b(c)", + error: null, + }, ]; diff --git a/tests/rustdoc-js-std/parser-weird-queries.js b/tests/rustdoc-js-std/parser-weird-queries.js index 26b8c32d68052..499b82a346948 100644 --- a/tests/rustdoc-js-std/parser-weird-queries.js +++ b/tests/rustdoc-js-std/parser-weird-queries.js @@ -37,15 +37,6 @@ const PARSED = [ userQuery: "a b", error: null, }, - { - query: 'a,b(c)', - elems: [], - foundElems: 0, - original: "a,b(c)", - returned: [], - userQuery: "a,b(c)", - error: "Expected `,`, `:` or `->`, found `(`", - }, { query: 'aaa,a', elems: [ diff --git a/tests/rustdoc-js/hof.js b/tests/rustdoc-js/hof.js index 1f5d2bfb6666c..5e6c9d83c7c7f 100644 --- a/tests/rustdoc-js/hof.js +++ b/tests/rustdoc-js/hof.js @@ -1,6 +1,12 @@ // exact-check const EXPECTED = [ + // not a HOF query + { + 'query': 'u32 -> !', + 'others': [], + }, + // ML-style higher-order function notation { 'query': 'bool, (u32 -> !) -> ()', @@ -53,11 +59,6 @@ const EXPECTED = [ {"path": "hof", "name": "fn_once"}, ], }, - { - 'query': 'u32 -> !', - // not a HOF query - 'others': [], - }, { 'query': '(str, str -> i8) -> ()', 'others': [ @@ -91,4 +92,85 @@ const EXPECTED = [ // params and return are not the same 'others': [], }, + + // Rust-style higher-order function notation + { + 'query': 'bool, fn(u32) -> ! -> ()', + 'others': [ + {"path": "hof", "name": "fn_ptr"}, + ], + }, + { + 'query': 'u8, fnonce(u32) -> ! -> ()', + 'others': [ + {"path": "hof", "name": "fn_once"}, + ], + }, + { + 'query': 'u8, fn(u32) -> ! -> ()', + // fnonce != fn + 'others': [], + }, + { + 'query': 'i8, fnmut(u32) -> ! -> ()', + 'others': [ + {"path": "hof", "name": "fn_mut"}, + ], + }, + { + 'query': 'i8, fn(u32) -> ! -> ()', + // fnmut != fn + 'others': [], + }, + { + 'query': 'char, fn(u32) -> ! -> ()', + 'others': [ + {"path": "hof", "name": "fn_"}, + ], + }, + { + 'query': 'char, fnmut(u32) -> ! -> ()', + // fn != fnmut + 'others': [], + }, + { + 'query': 'fn(first) -> ! -> ()', + 'others': [ + {"path": "hof", "name": "fn_ptr"}, + ], + }, + { + 'query': 'fnonce(second) -> ! -> ()', + 'others': [ + {"path": "hof", "name": "fn_once"}, + ], + }, + { + 'query': 'fnmut(third) -> ! -> ()', + 'others': [ + {"path": "hof", "name": "fn_mut"}, + ], + }, + { + 'query': 'fn(u32) -> ! -> ()', + 'others': [ + // fn matches primitive:fn and trait:Fn + {"path": "hof", "name": "fn_"}, + {"path": "hof", "name": "fn_ptr"}, + ], + }, + { + 'query': 'trait:fn(u32) -> ! -> ()', + 'others': [ + // fn matches primitive:fn and trait:Fn + {"path": "hof", "name": "fn_"}, + ], + }, + { + 'query': 'primitive:fn(u32) -> ! -> ()', + 'others': [ + // fn matches primitive:fn and trait:Fn + {"path": "hof", "name": "fn_ptr"}, + ], + }, ]; From c076509d8a0c685df1105bf81c65f5fe21c83aa8 Mon Sep 17 00:00:00 2001 From: "Celina G. Val" Date: Fri, 1 Mar 2024 11:00:33 -0800 Subject: [PATCH 15/33] Add methods to create constants I've been experimenting with transforming the StableMIR to instrument the code with potential UB checks. The modified body will only be used by our analysis tool, however, constants in StableMIR must be backed by rustc constants. Thus, I'm adding a few functions to build constants, such as building string and other primitives. --- compiler/rustc_smir/src/rustc_smir/context.rs | 58 ++++++++++++++++--- compiler/stable_mir/src/compiler_interface.rs | 17 ++++-- compiler/stable_mir/src/ty.rs | 27 ++++++++- 3 files changed, 90 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_smir/context.rs b/compiler/rustc_smir/src/rustc_smir/context.rs index 158d62a4830ff..26039a865f47f 100644 --- a/compiler/rustc_smir/src/rustc_smir/context.rs +++ b/compiler/rustc_smir/src/rustc_smir/context.rs @@ -23,7 +23,8 @@ use stable_mir::mir::Body; use stable_mir::target::{MachineInfo, MachineSize}; use stable_mir::ty::{ AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, ForeignDef, - ForeignItemKind, GenericArgs, LineInfo, PolyFnSig, RigidTy, Span, Ty, TyKind, VariantDef, + ForeignItemKind, GenericArgs, LineInfo, PolyFnSig, RigidTy, Span, Ty, TyKind, UintTy, + VariantDef, }; use stable_mir::{Crate, CrateDef, CrateItem, CrateNum, DefId, Error, Filename, ItemKind, Symbol}; use std::cell::RefCell; @@ -341,15 +342,56 @@ impl<'tcx> Context for TablesWrapper<'tcx> { .ok_or_else(|| Error::new(format!("Const `{cnst:?}` cannot be encoded as u64"))) } - fn usize_to_const(&self, val: u64) -> Result { + fn try_new_const_zst(&self, ty: Ty) -> Result { let mut tables = self.0.borrow_mut(); - let ty = tables.tcx.types.usize; + let tcx = tables.tcx; + let ty_internal = ty.internal(&mut *tables, tcx); + let size = tables + .tcx + .layout_of(ParamEnv::empty().and(ty_internal)) + .map_err(|err| { + Error::new(format!( + "Cannot create a zero-sized constant for type `{ty_internal}`: {err}" + )) + })? + .size; + if size.bytes() != 0 { + return Err(Error::new(format!( + "Cannot create a zero-sized constant for type `{ty_internal}`: \ + Type `{ty_internal}` has {} bytes", + size.bytes() + ))); + } + + Ok(ty::Const::zero_sized(tables.tcx, ty_internal).stable(&mut *tables)) + } + + fn new_const_str(&self, value: &str) -> Const { + let mut tables = self.0.borrow_mut(); + let tcx = tables.tcx; + let ty = ty::Ty::new_static_str(tcx); + let bytes = value.as_bytes(); + let val_tree = ty::ValTree::from_raw_bytes(tcx, bytes); + + ty::Const::new_value(tcx, val_tree, ty).stable(&mut *tables) + } + + fn new_const_bool(&self, value: bool) -> Const { + let mut tables = self.0.borrow_mut(); + ty::Const::from_bool(tables.tcx, value).stable(&mut *tables) + } + + fn try_new_const_uint(&self, value: u128, uint_ty: UintTy) -> Result { + let mut tables = self.0.borrow_mut(); + let tcx = tables.tcx; + let ty = ty::Ty::new_uint(tcx, uint_ty.internal(&mut *tables, tcx)); let size = tables.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap().size; - let scalar = ScalarInt::try_from_uint(val, size).ok_or_else(|| { - Error::new(format!("Value overflow: cannot convert `{val}` to usize.")) + // We don't use Const::from_bits since it doesn't have any error checking. + let scalar = ScalarInt::try_from_uint(value, size).ok_or_else(|| { + Error::new(format!("Value overflow: cannot convert `{value}` to `{ty}`.")) })?; - Ok(rustc_middle::ty::Const::new_value(tables.tcx, ValTree::from_scalar_int(scalar), ty) + Ok(ty::Const::new_value(tables.tcx, ValTree::from_scalar_int(scalar), ty) .stable(&mut *tables)) } @@ -556,7 +598,9 @@ impl<'tcx> Context for TablesWrapper<'tcx> { global_alloc: &GlobalAlloc, ) -> Option { let mut tables = self.0.borrow_mut(); - let GlobalAlloc::VTable(ty, trait_ref) = global_alloc else { return None }; + let GlobalAlloc::VTable(ty, trait_ref) = global_alloc else { + return None; + }; let tcx = tables.tcx; let alloc_id = tables.tcx.vtable_allocation(( ty.internal(&mut *tables, tcx), diff --git a/compiler/stable_mir/src/compiler_interface.rs b/compiler/stable_mir/src/compiler_interface.rs index 0f7d8d7e083bf..852d57ab14122 100644 --- a/compiler/stable_mir/src/compiler_interface.rs +++ b/compiler/stable_mir/src/compiler_interface.rs @@ -14,7 +14,7 @@ use crate::ty::{ AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, ForeignDef, ForeignItemKind, ForeignModule, ForeignModuleDef, GenericArgs, GenericPredicates, Generics, ImplDef, ImplTrait, LineInfo, PolyFnSig, RigidTy, Span, TraitDecl, TraitDef, Ty, TyKind, - VariantDef, + UintTy, VariantDef, }; use crate::{ mir, Crate, CrateItem, CrateItems, CrateNum, DefId, Error, Filename, ImplTraitDecls, ItemKind, @@ -101,8 +101,17 @@ pub trait Context { /// Evaluate constant as a target usize. fn eval_target_usize(&self, cnst: &Const) -> Result; - /// Create a target usize constant for the given value. - fn usize_to_const(&self, val: u64) -> Result; + /// Create a new zero-sized constant. + fn try_new_const_zst(&self, ty: Ty) -> Result; + + /// Create a new constant that represents the given string value. + fn new_const_str(&self, value: &str) -> Const; + + /// Create a new constant that represents the given boolean value. + fn new_const_bool(&self, value: bool) -> Const; + + /// Create a new constant that represents the given value. + fn try_new_const_uint(&self, value: u128, uint_ty: UintTy) -> Result; /// Create a new type from the given kind. fn new_rigid_ty(&self, kind: RigidTy) -> Ty; @@ -199,7 +208,7 @@ pub trait Context { // A thread local variable that stores a pointer to the tables mapping between TyCtxt // datastructures and stable MIR datastructures -scoped_thread_local! (static TLV: Cell<*const ()>); +scoped_thread_local!(static TLV: Cell<*const ()>); pub fn run(context: &dyn Context, f: F) -> Result where diff --git a/compiler/stable_mir/src/ty.rs b/compiler/stable_mir/src/ty.rs index 86cc748eaec93..a337675202871 100644 --- a/compiler/stable_mir/src/ty.rs +++ b/compiler/stable_mir/src/ty.rs @@ -128,13 +128,38 @@ impl Const { /// Creates an interned usize constant. fn try_from_target_usize(val: u64) -> Result { - with(|cx| cx.usize_to_const(val)) + with(|cx| cx.try_new_const_uint(val.into(), UintTy::Usize)) } /// Try to evaluate to a target `usize`. pub fn eval_target_usize(&self) -> Result { with(|cx| cx.eval_target_usize(self)) } + + /// Create a constant that represents a new zero-sized constant of type T. + /// Fails if the type is not a ZST or if it doesn't have a known size. + pub fn try_new_zero_sized(ty: Ty) -> Result { + with(|cx| cx.try_new_const_zst(ty)) + } + + /// Build a new constant that represents the given string. + /// + /// Note that there is no guarantee today about duplication of the same constant. + /// I.e.: Calling this function multiple times with the same argument may or may not return + /// the same allocation. + pub fn from_str(value: &str) -> Const { + with(|cx| cx.new_const_str(value)) + } + + /// Build a new constant that represents the given boolean value. + pub fn from_bool(value: bool) -> Const { + with(|cx| cx.new_const_bool(value)) + } + + /// Build a new constant that represents the given unsigned integer. + pub fn try_from_uint(value: u128, uint_ty: UintTy) -> Result { + with(|cx| cx.try_new_const_uint(value, uint_ty)) + } } #[derive(Clone, Copy, Debug, PartialEq, Eq)] From 893a9107b9f8b2038d77648a2ca4b06f36d3009d Mon Sep 17 00:00:00 2001 From: "Celina G. Val" Date: Tue, 12 Mar 2024 12:55:18 -0700 Subject: [PATCH 16/33] Add a test to SMIR body transformation --- .../ui-fulldeps/stable-mir/check_transform.rs | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 tests/ui-fulldeps/stable-mir/check_transform.rs diff --git a/tests/ui-fulldeps/stable-mir/check_transform.rs b/tests/ui-fulldeps/stable-mir/check_transform.rs new file mode 100644 index 0000000000000..e7d852a27df0e --- /dev/null +++ b/tests/ui-fulldeps/stable-mir/check_transform.rs @@ -0,0 +1,147 @@ +//@ run-pass +//! Test a few methods to transform StableMIR. + +//@ ignore-stage1 +//@ ignore-cross-compile +//@ ignore-remote +//@ ignore-windows-gnu mingw has troubles with linking https://github.com/rust-lang/rust/pull/116837 + +#![feature(rustc_private)] +#![feature(assert_matches)] +#![feature(control_flow_enum)] +#![feature(ascii_char, ascii_char_variants)] + +extern crate rustc_hir; +#[macro_use] +extern crate rustc_smir; +extern crate rustc_driver; +extern crate rustc_interface; +extern crate stable_mir; + +use rustc_smir::rustc_internal; +use stable_mir::mir::alloc::GlobalAlloc; +use stable_mir::mir::mono::Instance; +use stable_mir::mir::{Body, Constant, Operand, Rvalue, StatementKind, TerminatorKind}; +use stable_mir::ty::{Const, ConstantKind}; +use stable_mir::{CrateDef, CrateItems, ItemKind}; +use std::convert::TryFrom; +use std::io::Write; +use std::ops::ControlFlow; + +const CRATE_NAME: &str = "input"; + +/// This function uses the Stable MIR APIs to transform the MIR. +fn test_transform() -> ControlFlow<()> { + // Find items in the local crate. + let items = stable_mir::all_local_items(); + + // Test fn_abi + let target_fn = *get_item(&items, (ItemKind::Fn, "dummy")).unwrap(); + let instance = Instance::try_from(target_fn).unwrap(); + let body = instance.body().unwrap(); + check_msg(&body, "oops"); + + let new_msg = "new panic message"; + let new_body = change_panic_msg(body, new_msg); + check_msg(&new_body, new_msg); + + ControlFlow::Continue(()) +} + +/// Check that the body panic message matches the given message. +fn check_msg(body: &Body, expected: &str) { + let msg = body + .blocks + .iter() + .find_map(|bb| match &bb.terminator.kind { + TerminatorKind::Call { args, .. } => { + assert_eq!(args.len(), 1, "Expected panic message, but found {args:?}"); + let msg_const = match &args[0] { + Operand::Constant(msg_const) => msg_const, + Operand::Copy(place) | Operand::Move(place) => { + assert!(place.projection.is_empty()); + bb.statements + .iter() + .find_map(|stmt| match &stmt.kind { + StatementKind::Assign( + destination, + Rvalue::Use(Operand::Constant(msg_const)), + ) if destination == place => Some(msg_const), + _ => None, + }) + .unwrap() + } + }; + let ConstantKind::Allocated(alloc) = msg_const.literal.kind() else { + unreachable!() + }; + assert_eq!(alloc.provenance.ptrs.len(), 1); + + let alloc_prov_id = alloc.provenance.ptrs[0].1 .0; + let GlobalAlloc::Memory(val) = GlobalAlloc::from(alloc_prov_id) else { + unreachable!() + }; + let bytes = val.raw_bytes().unwrap(); + Some(std::str::from_utf8(&bytes).unwrap().to_string()) + } + _ => None, + }) + .expect("Failed to find panic message"); + assert_eq!(&msg, expected); +} + +/// Modify body to use a different panic message. +fn change_panic_msg(mut body: Body, new_msg: &str) -> Body { + for bb in &mut body.blocks { + match &mut bb.terminator.kind { + TerminatorKind::Call { args, .. } => { + let new_const = Const::from_str(new_msg); + args[0] = Operand::Constant(Constant { + literal: new_const, + span: bb.terminator.span, + user_ty: None, + }); + } + _ => {} + } + } + body +} + +fn get_item<'a>( + items: &'a CrateItems, + item: (ItemKind, &str), +) -> Option<&'a stable_mir::CrateItem> { + items.iter().find(|crate_item| (item.0 == crate_item.kind()) && crate_item.name() == item.1) +} + +/// This test will generate and analyze a dummy crate using the stable mir. +/// For that, it will first write the dummy crate into a file. +/// Then it will create a `StableMir` using custom arguments and then +/// it will run the compiler. +fn main() { + let path = "transform_input.rs"; + generate_input(&path).unwrap(); + let args = vec![ + "rustc".to_string(), + "--crate-type=lib".to_string(), + "--crate-name".to_string(), + CRATE_NAME.to_string(), + path.to_string(), + ]; + run!(args, test_transform).unwrap(); +} + +fn generate_input(path: &str) -> std::io::Result<()> { + let mut file = std::fs::File::create(path)?; + write!( + file, + r#" + #![feature(panic_internals)] + pub fn dummy() {{ + core::panicking::panic_str("oops"); + }} + "# + )?; + Ok(()) +} From a38a556ceb4b65c397a47bcbe35d004bc9b8626f Mon Sep 17 00:00:00 2001 From: ltdk Date: Tue, 12 Mar 2024 17:19:58 -0400 Subject: [PATCH 17/33] 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 18/33] 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 90acda134c0852411629a763e400615ae9f038b0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 13 Mar 2024 09:20:16 +0000 Subject: [PATCH 19/33] 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 20/33] 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 21/33] 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 22/33] 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 23/33] 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 24/33] 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 25/33] 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 26/33] 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 27/33] 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 28/33] 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 29/33] 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 339322735ebe9540102c6fdc96382610bee1e1ed Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 13 Mar 2024 09:25:20 +0000 Subject: [PATCH 30/33] 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 514b2745b35ecc8c8c0b8ec8c12bde3fa48cb464 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 13 Mar 2024 15:07:47 +0100 Subject: [PATCH 31/33] const-eval: organize and extend tests for required-consts --- .../consts/const-eval/erroneous-const.stderr | 15 ------- .../ui/consts/const-eval/erroneous-const2.rs | 19 --------- .../consts/const-eval/erroneous-const2.stderr | 15 ------- .../unused-broken-const-late.stderr | 11 ----- .../collect-in-called-fn.noopt.stderr | 17 ++++++++ .../collect-in-called-fn.opt.stderr | 17 ++++++++ .../collect-in-called-fn.rs} | 18 ++++---- .../collect-in-dead-drop.noopt.stderr | 14 +++++++ .../required-consts/collect-in-dead-drop.rs | 33 +++++++++++++++ .../collect-in-dead-fn.noopt.stderr | 17 ++++++++ .../required-consts/collect-in-dead-fn.rs | 35 ++++++++++++++++ .../required-consts/collect-in-dead-forget.rs | 32 +++++++++++++++ .../collect-in-dead-move.noopt.stderr | 14 +++++++ .../required-consts/collect-in-dead-move.rs | 33 +++++++++++++++ .../collect-in-dead-vtable.noopt.stderr | 17 ++++++++ .../required-consts/collect-in-dead-vtable.rs | 41 +++++++++++++++++++ .../interpret-in-const-called-fn.noopt.stderr | 17 ++++++++ .../interpret-in-const-called-fn.opt.stderr | 17 ++++++++ .../interpret-in-const-called-fn.rs} | 13 +++--- .../interpret-in-promoted.noopt.stderr | 27 ++++++++++++ .../interpret-in-promoted.opt.stderr | 27 ++++++++++++ .../required-consts/interpret-in-promoted.rs | 15 +++++++ .../interpret-in-static.noopt.stderr | 17 ++++++++ .../interpret-in-static.opt.stderr | 17 ++++++++ .../required-consts/interpret-in-static.rs | 21 ++++++++++ 25 files changed, 447 insertions(+), 72 deletions(-) delete mode 100644 tests/ui/consts/const-eval/erroneous-const.stderr delete mode 100644 tests/ui/consts/const-eval/erroneous-const2.rs delete mode 100644 tests/ui/consts/const-eval/erroneous-const2.stderr delete mode 100644 tests/ui/consts/const-eval/unused-broken-const-late.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-called-fn.noopt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-called-fn.opt.stderr rename tests/ui/consts/{const-eval/unused-broken-const-late.rs => required-consts/collect-in-called-fn.rs} (55%) create mode 100644 tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-drop.rs create mode 100644 tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-fn.rs create mode 100644 tests/ui/consts/required-consts/collect-in-dead-forget.rs create mode 100644 tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-move.rs create mode 100644 tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-vtable.rs create mode 100644 tests/ui/consts/required-consts/interpret-in-const-called-fn.noopt.stderr create mode 100644 tests/ui/consts/required-consts/interpret-in-const-called-fn.opt.stderr rename tests/ui/consts/{const-eval/erroneous-const.rs => required-consts/interpret-in-const-called-fn.rs} (52%) create mode 100644 tests/ui/consts/required-consts/interpret-in-promoted.noopt.stderr create mode 100644 tests/ui/consts/required-consts/interpret-in-promoted.opt.stderr create mode 100644 tests/ui/consts/required-consts/interpret-in-promoted.rs create mode 100644 tests/ui/consts/required-consts/interpret-in-static.noopt.stderr create mode 100644 tests/ui/consts/required-consts/interpret-in-static.opt.stderr create mode 100644 tests/ui/consts/required-consts/interpret-in-static.rs diff --git a/tests/ui/consts/const-eval/erroneous-const.stderr b/tests/ui/consts/const-eval/erroneous-const.stderr deleted file mode 100644 index bd25e96c2cf13..0000000000000 --- a/tests/ui/consts/const-eval/erroneous-const.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0080]: evaluation of `PrintName::::VOID` failed - --> $DIR/erroneous-const.rs:6:22 - | -LL | const VOID: () = [()][2]; - | ^^^^^^^ index out of bounds: the length is 1 but the index is 2 - -note: erroneous constant encountered - --> $DIR/erroneous-const.rs:13:13 - | -LL | PrintName::::VOID; - | ^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/const-eval/erroneous-const2.rs b/tests/ui/consts/const-eval/erroneous-const2.rs deleted file mode 100644 index 61f2955f2d822..0000000000000 --- a/tests/ui/consts/const-eval/erroneous-const2.rs +++ /dev/null @@ -1,19 +0,0 @@ -//! Make sure we error on erroneous consts even if they are unused. -#![allow(unconditional_panic)] - -struct PrintName(T); -impl PrintName { - const VOID: () = [()][2]; //~ERROR evaluation of `PrintName::::VOID` failed -} - -pub static FOO: () = { - if false { - // This bad constant is only used in dead code in a static initializer... and yet we still - // must make sure that the build fails. - PrintName::::VOID; //~ constant - } -}; - -fn main() { - FOO -} diff --git a/tests/ui/consts/const-eval/erroneous-const2.stderr b/tests/ui/consts/const-eval/erroneous-const2.stderr deleted file mode 100644 index 6a5839e3dfb41..0000000000000 --- a/tests/ui/consts/const-eval/erroneous-const2.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0080]: evaluation of `PrintName::::VOID` failed - --> $DIR/erroneous-const2.rs:6:22 - | -LL | const VOID: () = [()][2]; - | ^^^^^^^ index out of bounds: the length is 1 but the index is 2 - -note: erroneous constant encountered - --> $DIR/erroneous-const2.rs:13:9 - | -LL | PrintName::::VOID; - | ^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/const-eval/unused-broken-const-late.stderr b/tests/ui/consts/const-eval/unused-broken-const-late.stderr deleted file mode 100644 index c2cf2f3813c5e..0000000000000 --- a/tests/ui/consts/const-eval/unused-broken-const-late.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0080]: evaluation of `PrintName::::VOID` failed - --> $DIR/unused-broken-const-late.rs:8:22 - | -LL | const VOID: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/unused-broken-const-late.rs:8:22 - | - = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-called-fn.noopt.stderr b/tests/ui/consts/required-consts/collect-in-called-fn.noopt.stderr new file mode 100644 index 0000000000000..c7ff1328917fc --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-called-fn.noopt.stderr @@ -0,0 +1,17 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-called-fn.rs:9:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-called-fn.rs:9:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: the above error was encountered while instantiating `fn called::` + --> $DIR/collect-in-called-fn.rs:23:5 + | +LL | called::(); + | ^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-called-fn.opt.stderr b/tests/ui/consts/required-consts/collect-in-called-fn.opt.stderr new file mode 100644 index 0000000000000..c7ff1328917fc --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-called-fn.opt.stderr @@ -0,0 +1,17 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-called-fn.rs:9:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-called-fn.rs:9:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: the above error was encountered while instantiating `fn called::` + --> $DIR/collect-in-called-fn.rs:23:5 + | +LL | called::(); + | ^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/const-eval/unused-broken-const-late.rs b/tests/ui/consts/required-consts/collect-in-called-fn.rs similarity index 55% rename from tests/ui/consts/const-eval/unused-broken-const-late.rs rename to tests/ui/consts/required-consts/collect-in-called-fn.rs index c4916061f9e5a..55133a10cd988 100644 --- a/tests/ui/consts/const-eval/unused-broken-const-late.rs +++ b/tests/ui/consts/required-consts/collect-in-called-fn.rs @@ -1,20 +1,24 @@ +//@revisions: noopt opt //@ build-fail -//@ compile-flags: -O +//@[opt] compile-flags: -O //! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is //! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) -struct PrintName(T); -impl PrintName { - const VOID: () = panic!(); //~ERROR evaluation of `PrintName::::VOID` failed +struct Fail(T); +impl Fail { + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed } -fn no_codegen() { +#[inline(never)] +fn called() { // Any function that is called is guaranteed to have all consts that syntactically // appear in its body evaluated, even if they only appear in dead code. + // This relies on mono-item collection checking `required_consts` in collected functions. if false { - let _ = PrintName::::VOID; + let _ = Fail::::C; } } + pub fn main() { - no_codegen::(); + called::(); } diff --git a/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr new file mode 100644 index 0000000000000..b7010e787633b --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr @@ -0,0 +1,14 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-drop.rs:12:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-drop.rs:12:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: the above error was encountered while instantiating `fn as std::ops::Drop>::drop` + --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-drop.rs b/tests/ui/consts/required-consts/collect-in-dead-drop.rs new file mode 100644 index 0000000000000..c9ffcec690317 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-drop.rs @@ -0,0 +1,33 @@ +//@revisions: noopt opt +//@[noopt] build-fail +//@[opt] compile-flags: -O +//FIXME: `opt` revision currently does not stop with an error due to +//. +//@[opt] build-pass +//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is +//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) + +struct Fail(T); +impl Fail { + const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::::C` failed +} + +// This function is not actually called, but is mentioned implicitly as destructor in dead code in a +// function that is called. Make sure we still find this error. +impl Drop for Fail { + fn drop(&mut self) { + let _ = Fail::::C; + } +} + +#[inline(never)] +fn called(x: T) { + if false { + let v = Fail(x); + // Now it gest dropped implicitly, at the end of this scope. + } +} + +pub fn main() { + called::(0); +} diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr new file mode 100644 index 0000000000000..2162c35c83702 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr @@ -0,0 +1,17 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-fn.rs:12:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn.rs:12:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: the above error was encountered while instantiating `fn not_called::` + --> $DIR/collect-in-dead-fn.rs:29:9 + | +LL | not_called::(); + | ^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn.rs b/tests/ui/consts/required-consts/collect-in-dead-fn.rs new file mode 100644 index 0000000000000..9e6b151915331 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-fn.rs @@ -0,0 +1,35 @@ +//@revisions: noopt opt +//@[noopt] build-fail +//@[opt] compile-flags: -O +//FIXME: `opt` revision currently does not stop with an error due to +//. +//@[opt] build-pass +//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is +//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) + +struct Fail(T); +impl Fail { + const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::::C` failed +} + +// This function is not actually called, but it is mentioned in dead code in a function that is +// called. Make sure we still find this error. +// This relies on mono-item collection checking `required_consts` in functions that syntactically +// are called in collected functions (even inside dead code). +#[inline(never)] +fn not_called() { + if false { + let _ = Fail::::C; + } +} + +#[inline(never)] +fn called() { + if false { + not_called::(); + } +} + +pub fn main() { + called::(); +} diff --git a/tests/ui/consts/required-consts/collect-in-dead-forget.rs b/tests/ui/consts/required-consts/collect-in-dead-forget.rs new file mode 100644 index 0000000000000..720b7a499f781 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-forget.rs @@ -0,0 +1,32 @@ +//@revisions: noopt opt +//@build-pass +//@[opt] compile-flags: -O +//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is +//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) + +struct Fail(T); +impl Fail { + const C: () = panic!(); +} + +// This function is not actually called, but is mentioned implicitly as destructor in dead code in a +// function that is called. Make sure we still find this error. +impl Drop for Fail { + fn drop(&mut self) { + let _ = Fail::::C; + } +} + +#[inline(never)] +fn called(x: T) { + if false { + let v = Fail(x); + std::mem::forget(v); + // Now the destructor never gets "mentioned" so this build should *not* fail. + // IOW, this demonstrates that we are using a post-drop-elab notion of "mentioned". + } +} + +pub fn main() { + called::(0); +} diff --git a/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr new file mode 100644 index 0000000000000..8c853127e044c --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr @@ -0,0 +1,14 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-move.rs:12:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-move.rs:12:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: the above error was encountered while instantiating `fn as std::ops::Drop>::drop` + --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-move.rs b/tests/ui/consts/required-consts/collect-in-dead-move.rs new file mode 100644 index 0000000000000..f3a6ba8a6577c --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-move.rs @@ -0,0 +1,33 @@ +//@revisions: noopt opt +//@[noopt] build-fail +//@[opt] compile-flags: -O +//FIXME: `opt` revision currently does not stop with an error due to +//. +//@[opt] build-pass +//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is +//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) + +struct Fail(T); +impl Fail { + const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::::C` failed +} + +// This function is not actually called, but is mentioned implicitly as destructor in dead code in a +// function that is called. Make sure we still find this error. +impl Drop for Fail { + fn drop(&mut self) { + let _ = Fail::::C; + } +} + +#[inline(never)] +fn called(x: T) { + if false { + let v = Fail(x); + drop(v); // move `v` away (and it then gets dropped there so build still fails) + } +} + +pub fn main() { + called::(0); +} diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr new file mode 100644 index 0000000000000..6fd82777bd364 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr @@ -0,0 +1,17 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-vtable.rs:12:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:12:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: the above error was encountered while instantiating `fn as MyTrait>::not_called` + --> $DIR/collect-in-dead-vtable.rs:35:40 + | +LL | let gen_vtable: &dyn MyTrait = &v; // vtable "appears" here + | ^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.rs b/tests/ui/consts/required-consts/collect-in-dead-vtable.rs new file mode 100644 index 0000000000000..f21a1cc1fc2fd --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.rs @@ -0,0 +1,41 @@ +//@revisions: noopt opt +//@[noopt] build-fail +//@[opt] compile-flags: -O +//FIXME: `opt` revision currently does not stop with an error due to +//. +//@[opt] build-pass +//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is +//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) + +struct Fail(T); +impl Fail { + const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::::C` failed +} + +trait MyTrait { + fn not_called(&self); +} + +// This function is not actually called, but it is mentioned in a vtable in a function that is +// called. Make sure we still find this error. +// This relies on mono-item collection checking `required_consts` in functions that are referenced +// in vtables that syntactically appear in collected functions (even inside dead code). +impl MyTrait for Vec { + fn not_called(&self) { + if false { + let _ = Fail::::C; + } + } +} + +#[inline(never)] +fn called() { + if false { + let v: Vec = Vec::new(); + let gen_vtable: &dyn MyTrait = &v; // vtable "appears" here + } +} + +pub fn main() { + called::(); +} diff --git a/tests/ui/consts/required-consts/interpret-in-const-called-fn.noopt.stderr b/tests/ui/consts/required-consts/interpret-in-const-called-fn.noopt.stderr new file mode 100644 index 0000000000000..75304591b9f05 --- /dev/null +++ b/tests/ui/consts/required-consts/interpret-in-const-called-fn.noopt.stderr @@ -0,0 +1,17 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/interpret-in-const-called-fn.rs:7:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/interpret-in-const-called-fn.rs:7:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/interpret-in-const-called-fn.rs:16:9 + | +LL | Fail::::C; + | ^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/interpret-in-const-called-fn.opt.stderr b/tests/ui/consts/required-consts/interpret-in-const-called-fn.opt.stderr new file mode 100644 index 0000000000000..75304591b9f05 --- /dev/null +++ b/tests/ui/consts/required-consts/interpret-in-const-called-fn.opt.stderr @@ -0,0 +1,17 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/interpret-in-const-called-fn.rs:7:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/interpret-in-const-called-fn.rs:7:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/interpret-in-const-called-fn.rs:16:9 + | +LL | Fail::::C; + | ^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/const-eval/erroneous-const.rs b/tests/ui/consts/required-consts/interpret-in-const-called-fn.rs similarity index 52% rename from tests/ui/consts/const-eval/erroneous-const.rs rename to tests/ui/consts/required-consts/interpret-in-const-called-fn.rs index 74d44c5259a3e..c409fae0bb905 100644 --- a/tests/ui/consts/const-eval/erroneous-const.rs +++ b/tests/ui/consts/required-consts/interpret-in-const-called-fn.rs @@ -1,16 +1,19 @@ +//@revisions: noopt opt +//@[opt] compile-flags: -O //! Make sure we error on erroneous consts even if they are unused. -#![allow(unconditional_panic)] -struct PrintName(T); -impl PrintName { - const VOID: () = [()][2]; //~ERROR evaluation of `PrintName::::VOID` failed +struct Fail(T); +impl Fail { + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed } +#[inline(never)] const fn no_codegen() { if false { // This bad constant is only used in dead code in a no-codegen function... and yet we still // must make sure that the build fails. - PrintName::::VOID; //~ constant + // This relies on const-eval evaluating all `required_consts` of `const fn`. + Fail::::C; //~ constant } } diff --git a/tests/ui/consts/required-consts/interpret-in-promoted.noopt.stderr b/tests/ui/consts/required-consts/interpret-in-promoted.noopt.stderr new file mode 100644 index 0000000000000..491131daf8de4 --- /dev/null +++ b/tests/ui/consts/required-consts/interpret-in-promoted.noopt.stderr @@ -0,0 +1,27 @@ +error[E0080]: evaluation of constant value failed + --> $SRC_DIR/core/src/hint.rs:LL:COL + | + = note: entering unreachable code + | +note: inside `unreachable_unchecked` + --> $SRC_DIR/core/src/hint.rs:LL:COL +note: inside `ub` + --> $DIR/interpret-in-promoted.rs:6:5 + | +LL | std::hint::unreachable_unchecked(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: inside `FOO` + --> $DIR/interpret-in-promoted.rs:12:28 + | +LL | let _x: &'static () = &ub(); + | ^^^^ + +note: erroneous constant encountered + --> $DIR/interpret-in-promoted.rs:12:27 + | +LL | let _x: &'static () = &ub(); + | ^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/interpret-in-promoted.opt.stderr b/tests/ui/consts/required-consts/interpret-in-promoted.opt.stderr new file mode 100644 index 0000000000000..491131daf8de4 --- /dev/null +++ b/tests/ui/consts/required-consts/interpret-in-promoted.opt.stderr @@ -0,0 +1,27 @@ +error[E0080]: evaluation of constant value failed + --> $SRC_DIR/core/src/hint.rs:LL:COL + | + = note: entering unreachable code + | +note: inside `unreachable_unchecked` + --> $SRC_DIR/core/src/hint.rs:LL:COL +note: inside `ub` + --> $DIR/interpret-in-promoted.rs:6:5 + | +LL | std::hint::unreachable_unchecked(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: inside `FOO` + --> $DIR/interpret-in-promoted.rs:12:28 + | +LL | let _x: &'static () = &ub(); + | ^^^^ + +note: erroneous constant encountered + --> $DIR/interpret-in-promoted.rs:12:27 + | +LL | let _x: &'static () = &ub(); + | ^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/interpret-in-promoted.rs b/tests/ui/consts/required-consts/interpret-in-promoted.rs new file mode 100644 index 0000000000000..9c2cf4e70d3fb --- /dev/null +++ b/tests/ui/consts/required-consts/interpret-in-promoted.rs @@ -0,0 +1,15 @@ +//@revisions: noopt opt +//@[opt] compile-flags: -O +//! Make sure we error on erroneous consts even if they are unused. + +const unsafe fn ub() { + std::hint::unreachable_unchecked(); +} + +pub const FOO: () = unsafe { + // Make sure that this gets promoted and then fails to evaluate, and we deal with that + // correctly. + let _x: &'static () = &ub(); //~ erroneous constant +}; + +fn main() {} diff --git a/tests/ui/consts/required-consts/interpret-in-static.noopt.stderr b/tests/ui/consts/required-consts/interpret-in-static.noopt.stderr new file mode 100644 index 0000000000000..159c9449fc041 --- /dev/null +++ b/tests/ui/consts/required-consts/interpret-in-static.noopt.stderr @@ -0,0 +1,17 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/interpret-in-static.rs:7:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/interpret-in-static.rs:7:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/interpret-in-static.rs:15:9 + | +LL | Fail::::C; + | ^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/interpret-in-static.opt.stderr b/tests/ui/consts/required-consts/interpret-in-static.opt.stderr new file mode 100644 index 0000000000000..159c9449fc041 --- /dev/null +++ b/tests/ui/consts/required-consts/interpret-in-static.opt.stderr @@ -0,0 +1,17 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/interpret-in-static.rs:7:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/interpret-in-static.rs:7:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/interpret-in-static.rs:15:9 + | +LL | Fail::::C; + | ^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/interpret-in-static.rs b/tests/ui/consts/required-consts/interpret-in-static.rs new file mode 100644 index 0000000000000..559e281b2b038 --- /dev/null +++ b/tests/ui/consts/required-consts/interpret-in-static.rs @@ -0,0 +1,21 @@ +//@revisions: noopt opt +//@[opt] compile-flags: -O +//! Make sure we error on erroneous consts even if they are unused. + +struct Fail(T); +impl Fail { + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed +} + +pub static FOO: () = { + if false { + // This bad constant is only used in dead code in a static initializer... and yet we still + // must make sure that the build fails. + // This relies on const-eval evaluating all `required_consts` of the `static` MIR body. + Fail::::C; //~ constant + } +}; + +fn main() { + FOO +} From be33586adc13444e7d08c4269344db2dce6c2a03 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Wed, 13 Mar 2024 22:38:05 +0100 Subject: [PATCH 32/33] fix unsoundness in Step::forward_unchecked for signed integers --- library/core/src/iter/range.rs | 30 ++++++++++++++++++++++++++++-- library/core/tests/iter/range.rs | 5 +++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/library/core/src/iter/range.rs b/library/core/src/iter/range.rs index 68937161e046a..49135704f27fa 100644 --- a/library/core/src/iter/range.rs +++ b/library/core/src/iter/range.rs @@ -184,8 +184,25 @@ pub trait Step: Clone + PartialOrd + Sized { } } -// These are still macro-generated because the integer literals resolve to different types. -macro_rules! step_identical_methods { +// Separate impls for signed ranges because the distance within a signed range can be larger +// than the signed::MAX value. Therefore `as` casting to the signed type would be incorrect. +macro_rules! step_signed_methods { + ($unsigned: ty) => { + #[inline] + unsafe fn forward_unchecked(start: Self, n: usize) -> Self { + // SAFETY: the caller has to guarantee that `start + n` doesn't overflow. + unsafe { start.checked_add_unsigned(n as $unsigned).unwrap_unchecked() } + } + + #[inline] + unsafe fn backward_unchecked(start: Self, n: usize) -> Self { + // SAFETY: the caller has to guarantee that `start - n` doesn't overflow. + unsafe { start.checked_sub_unsigned(n as $unsigned).unwrap_unchecked() } + } + }; +} + +macro_rules! step_unsigned_methods { () => { #[inline] unsafe fn forward_unchecked(start: Self, n: usize) -> Self { @@ -198,7 +215,12 @@ macro_rules! step_identical_methods { // SAFETY: the caller has to guarantee that `start - n` doesn't overflow. unsafe { start.unchecked_sub(n as Self) } } + }; +} +// These are still macro-generated because the integer literals resolve to different types. +macro_rules! step_identical_methods { + () => { #[inline] #[allow(arithmetic_overflow)] #[rustc_inherit_overflow_checks] @@ -239,6 +261,7 @@ macro_rules! step_integer_impls { #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] impl Step for $u_narrower { step_identical_methods!(); + step_unsigned_methods!(); #[inline] fn steps_between(start: &Self, end: &Self) -> Option { @@ -271,6 +294,7 @@ macro_rules! step_integer_impls { #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] impl Step for $i_narrower { step_identical_methods!(); + step_signed_methods!($u_narrower); #[inline] fn steps_between(start: &Self, end: &Self) -> Option { @@ -335,6 +359,7 @@ macro_rules! step_integer_impls { #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] impl Step for $u_wider { step_identical_methods!(); + step_unsigned_methods!(); #[inline] fn steps_between(start: &Self, end: &Self) -> Option { @@ -360,6 +385,7 @@ macro_rules! step_integer_impls { #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] impl Step for $i_wider { step_identical_methods!(); + step_signed_methods!($u_wider); #[inline] fn steps_between(start: &Self, end: &Self) -> Option { diff --git a/library/core/tests/iter/range.rs b/library/core/tests/iter/range.rs index 9af07119a89a2..e31db0732e094 100644 --- a/library/core/tests/iter/range.rs +++ b/library/core/tests/iter/range.rs @@ -325,6 +325,11 @@ fn test_range_advance_by() { assert_eq!(Ok(()), r.advance_back_by(usize::MAX)); assert_eq!((r.start, r.end), (0u128 + usize::MAX as u128, u128::MAX - usize::MAX as u128)); + + // issue 122420, Step::forward_unchecked was unsound for signed integers + let mut r = -128i8..127; + assert_eq!(Ok(()), r.advance_by(200)); + assert_eq!(r.next(), Some(72)); } #[test] From d3cab9f4d6c778da99185a4548ef7f0899f04766 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Thu, 14 Mar 2024 00:57:59 +0100 Subject: [PATCH 33/33] update virtual clock in miri test since signed loops now execute more methods --- src/tools/miri/tests/pass/shims/time-with-isolation2.stdout | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout b/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout index c68b40b744bf2..dce51a7fdbe0f 100644 --- a/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout +++ b/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout @@ -1 +1 @@ -The loop took around 7s +The loop took around 12s