rustup and small fixes by oli-obk · Pull Request #91 · rust-lang/miri (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Choose a reason for hiding this comment

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

Nice work

// current stack.
let first = self.value_to_ptr_dont_use(args[0].0, args[0].1)?;
args[0].0 = Value::ByVal(PrimVal::from_ptr(first));
let ptr = match args[0].0 {

Choose a reason for hiding this comment

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

This code makes every element of temporaries a Value::ByRef. Can we just make temporaries into a Vec<Pointer>?

The way I see it, no accesses go through temporaries and they're only used for cleanup so it makes sense to be a list of Pointers. (Heck, even just a list of allocation ids should do it.)

Choose a reason for hiding this comment

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

done (stayed with Pointers, since allocate returns a Pointer and deallocate takes one)

I'm getting

Compiling miri v0.1.0 (https://github.com/solson/miri.git#b96202b3)
error[E0107]: wrong number of lifetime parameters: expected 0, found 1
    --> /home/xxx/.cargo/git/checkouts/miri-f9dcdd0c0ec69fb1/b96202b/src/interpreter/mod.rs:1801:76
     |
1801 | pub fn monomorphize_field_ty<'a, 'tcx:'a >(tcx: TyCtxt<'a, 'tcx, 'tcx>, f: ty::FieldDef<'tcx>, substs: &'tcx Substs<'tcx>) -> Ty<'tcx> {
     |                                                                            ^^^^^^^^^^^^^^^^^^ unexpected lifetime parameter

error[E0107]: wrong number of lifetime parameters: expected 0, found 1
   --> /home/xxx/.cargo/git/checkouts/miri-f9dcdd0c0ec69fb1/b96202b/src/interpreter/terminator/intrinsics.rs:492:12
    |
492 |         f: ty::FieldDef<'tcx>,
    |            ^^^^^^^^^^^^^^^^^^ unexpected lifetime parameter

error: aborting due to 2 previous errors

error: Could not compile `miri`.

I believe this PR fixes that. Can it be merged ASAP?

@ashleysommer I cherry-picked the commit that fixes that into master. It should build for you now.

/// Temporaries introduced to save stackframes
/// This is pure interpreter magic and has nothing to do with how rustc does it
/// An example is calling an FnMut closure that has been converted to a FnOnce closure
/// If they are Value::ByRef, their memory will be freed when the stackframe finishes

Choose a reason for hiding this comment

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

This comment is outdated now.

github-actions bot pushed a commit that referenced this pull request

Apr 18, 2025

Introduce and use specialized //@ ignore-auxiliary for test support files instead of using //@ ignore-test

Summary

Add a semantically meaningful directive for ignoring test auxiliary files. This is for auxiliary files that participate in actual tests but should not be built by compiletest (i.e. these files are involved through mod xxx; or include!() or #[path = "xxx"], etc.).

Motivation

A specialized directive like //@ ignore-auxiliary makes it way easier to audit disabled tests via //@ ignore-test.

These support files cannot use the canonical auxiliary/ dir because they participate in module resolution or are included, or their relative paths can be important for test intention otherwise.

Follow-up to:

#139705
#139783
#139740

Remarks on remaining unconditionally disabled tests under `tests/`

After this PR, against commit 79a272c6402, only 14 remaining test files are disabled through //@ ignore-test:

Remaining `//@ ignore-test` files under `tests/`

tests/debuginfo/drop-locations.rs
4://@ ignore-test (broken, see #128971)

tests/rustdoc/macro-document-private-duplicate.rs
1://@ ignore-test (fails spuriously, see issue #89228)

tests/rustdoc/inline_cross/assoc-const-equality.rs
3://@ ignore-test (FIXME: #125092)

tests/ui/match/issue-27021.rs
7://@ ignore-test (#54987)

tests/ui/match/issue-26996.rs
7://@ ignore-test (#54987)

tests/ui/issues/issue-49298.rs
9://@ ignore-test (#54987)

tests/ui/issues/issue-59756.rs
2://@ ignore-test (rustfix needs multiple suggestions)

tests/ui/precondition-checks/write.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/read.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/write_bytes.rs
5://@ ignore-test (unimplemented)

tests/ui/explicit-tail-calls/drop-order.rs
2://@ ignore-test: tail calls are not implemented in rustc_codegen_ssa yet, so this causes 🧊

tests/ui/panics/panic-short-backtrace-windows-x86_64.rs
3://@ ignore-test (#92000)

tests/ui/json/json-bom-plus-crlf-multifile-aux.rs
3://@ ignore-test Not a test. Used by other tests

tests/ui/traits/next-solver/object-soundness-requires-generalization.rs
2://@ ignore-test (see #114196)

Of these, most are either unimplemented, or spurious, or known-broken. The outstanding one is tests/ui/json/json-bom-plus-crlf-multifile-aux.rs which I did not want to touch in this PR -- that aux file has load-bearing BOM and carriage returns and byte offset matters. I think those test files that require special encoding / BOM probably are better off as run-make tests. See #139968 for that aux file.

Review advice

Best reviewed commit-by-commit.
The directive name diverged from the most voted //@ auxiliary because I think that's easy to confuse with //@ aux-{crate,dir}.

r? compiler

github-actions bot pushed a commit that referenced this pull request

Nov 4, 2025

…, r=scottmcm

Make explicit that TypeId's layout and size are unstable

Or worded differently, explicitly remark non-stable-guarantee of TypeId layout and size.

This PR makes no additional guarantees or non-guarantees, it only emphasizes that TypeId's size and layout are unstable like any other #[repr(Rust)] types.

This was discussed during #t-compiler/meetings > [weekly] 2025-10-30 @ 💬, where the compiler team discussed a request rust-lang/rust#148265 to have the standard library (and language) commit to TypeId guaranteeing a size upper bound of 16 bytes. In the meeting, the consensus was:

We were sympathetic to the use case discussed in the request PR, however we feel like this stability guarantee is premature, given that there are unresolved questions surrounding the intended purpose of TypeId, and concerns surrounding its collision-resistance properties rust-lang/rust#10389 and rust-lang/rust#129014. We would prefer not making any of such guarantee until the collision-resistance concerns are resolved.
Committing to a stability guarantee on the size upper bound now would close the door to making TypeId larger (even if unlikely for perf reasons).

Given that we have previously broken people who asserted the size of TypeId is 8 bytes, it was also discussed in the meeting that we should explicitly note that the size and layout of TypeId is not a stable guarantee, and is subject to changes between Rust releases, and thus cannot be relied upon -- if breakage in people's code is due to that assumption, it will be considered a won't-fix.

So even if #[repr(Rust)] types have unstable size and layout, this PR makes it explicit for TypeId since this type can feel "special" and users can be lead into thinking its size and layout is something they can rely upon.

r? @scottmcm (or libs/libs-api/lang)

rustup and small fixes by oli-obk · Pull Request #91 · rust-lang/miri (original) (raw)

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Summary

Motivation

Remarks on remaining unconditionally disabled tests under tests/

Review advice

Remarks on remaining unconditionally disabled tests under `tests/`