Skip to content

Conversation

@qsctr
Copy link
Collaborator

@qsctr qsctr commented Dec 1, 2025

There are three parts to this PR:

  1. Change the toolchain version and get mir-json to work with the new toolchain
  2. Replace the patched standard libraries with the unpatched versions from the new toolchain
  3. Re-apply patches to the new standard libraries (and any new patches necessary)

The vast majority of lines changed are from part 2 and are not interesting, so if reviewing the diff make sure to exclude those.

There were no major changes to mir-json prompted by the toolchain upgrade. Most of the changes were to fix compile errors caused by upstream refactoring; there was one change to runtime behavior (94ceb95) to deal with the new TypeId constants. The schema did not change at a level of detail captured by mir-json-schema.ts (for instance, new variants were added to Abi, but mir-json-schema.ts just describes it as { kind: string, ... }), so this may actually be backwards-compatible, but to be safe I bumped the schema version anyways.

Most of the patches had to be re-applied. 8cff259 didn't need to be re-applied, because upstream apparently changed to an implementation that was faster and less unsafe! There was one entirely new patch (67ffc40) and one patch that needed to be done in a different way (7b67f3e). Several other patches had to be somewhat modified as well.

qsctr added 30 commits November 14, 2025 00:50
@qsctr qsctr self-assigned this Dec 1, 2025
Copy link
Contributor

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooray! Have you tested this against crux-mir, out of curiosity?

The Ubuntu CI job is broken due to changes to the addition of the into_timespec function (in rust-lang/rust#141829), which we need to implement for sys::pal::crux::time::Instant.

Comment on lines 1535 to 1538
return Some(json!({
"kind": "raw_ptr",
"val": offset.bytes().to_string(),
}))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is identical to the JSON output for the if prov.is_none() case above, right? Would it be worth factoring this out into its own definition to avoid code duplication?

@qsctr
Copy link
Collaborator Author

qsctr commented Dec 1, 2025

Hooray! Have you tested this against crux-mir, out of curiosity?

Yep, I'll put up the corresponding crucible PR soon.

The Ubuntu CI job is broken due to changes to the addition of the into_timespec function (in rust-lang/rust#141829), which we need to implement for sys::pal::crux::time::Instant.

I'm tempted to instead remove the single use of into_timespec, as it's only used as part of an optimization to do more precise sleeping on certain platforms. Our time implementation is entirely fake anyways so I think we could just fall back to the Self::sleep case.

@RyanGlScott
Copy link
Contributor

I'm tempted to instead remove the single use of into_timespec, as it's only used as part of an optimization to do more precise sleeping on certain platforms. Our time implementation is entirely fake anyways so I think we could just fall back to the Self::sleep case.

I would be OK with that, especially since into_timespec doesn't appear to be part of the public API.

@qsctr qsctr merged commit da4ca75 into master Dec 1, 2025
5 checks passed
@qsctr qsctr deleted the rust-1.91 branch December 1, 2025 21:57
RyanGlScott added a commit that referenced this pull request Dec 2, 2025
This re-applies the changes from #154, which were omitted from #200 due to an
oversight. It is a bit unclear whether or not `#[inline(never)]` is strictly
necessary here (see
#153 (comment)), but
better to be on the safe side.

To make it more likely that we will remember to apply this change in the
future, I have started a "Notes" section in `libs/Patches.md` that very
explicitly calls out the needs for `#[inline(never)]`, and I have cited this
note from the relevant patch descriptions. Going forward, we can use this
"Notes" section to describe other aspects of patches that require more in-depth
explanations that what can be afforded in the brief patch descriptions.
RyanGlScott added a commit that referenced this pull request Dec 3, 2025
This re-applies the changes from #154, which were omitted from #200 due to an
oversight. It is a bit unclear whether or not `#[inline(never)]` is strictly
necessary here (see
#153 (comment)), but
better to be on the safe side.

To make it more likely that we will remember to apply this change in the
future, I have started a "Notes" section in `libs/Patches.md` that very
explicitly calls out the needs for `#[inline(never)]`, and I have cited this
note from the relevant patch descriptions. Going forward, we can use this
"Notes" section to describe other aspects of patches that require more in-depth
explanations that what can be afforded in the brief patch descriptions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants