Skip to content

Make target pointer width in target json an integer #144443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

WaffleLapkin
Copy link
Member

@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2025

Noratrieb is not on the review rotation at the moment.
They may take a while to respond.

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 25, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2025

This PR modifies run-make tests.

cc @jieyouxu

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred in src/tools/compiletest

cc @jieyouxu

Copy link
Member

@Noratrieb Noratrieb left a comment

Choose a reason for hiding this comment

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

I'm unsure if it's worth breaking everyone over such a minor improvement.. maybe? maybe not?

@rust-log-analyzer

This comment has been minimized.

@Noratrieb
Copy link
Member

Noratrieb commented Jul 25, 2025

I'm inclined to say yes, but it would probably be good to get this merged today so it can be in the same nightly as the other breaking change instead of two consecutive nightlies (it would also be fine to do that, but less cool)

@RalfJung
Copy link
Member

I'm unsure if it's worth breaking everyone over such a minor improvement.. maybe? maybe not?

We already did that with #144443... the current state is just inconsistent.

But, no strong opinion.

@WaffleLapkin WaffleLapkin force-pushed the integer-target-pointer-width branch from bb5b5c5 to 283f21b Compare July 25, 2025 11:55
@WaffleLapkin
Copy link
Member Author

@Noratrieb what's the other breaking change? #142352 has been merged for over a month..

@rust-log-analyzer

This comment has been minimized.

@WaffleLapkin WaffleLapkin force-pushed the integer-target-pointer-width branch from 283f21b to 267f0bd Compare July 25, 2025 12:35
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@RalfJung
Copy link
Member

RalfJung commented Jul 25, 2025

@Noratrieb what's the other breaking change? #142352 has been merged for over a month..

Uh, but then why did Miri CI only start failing today?

Something changed in 9748d87...b56aaec.

@WaffleLapkin WaffleLapkin force-pushed the integer-target-pointer-width branch from b64ab4a to b2871e5 Compare July 25, 2025 13:33
@RalfJung
Copy link
Member

RalfJung commented Jul 25, 2025

The breaking change was probably #144218.

@WaffleLapkin
Copy link
Member Author

The only relevant change I see is #144218, but that doesn't explain anything, it should have been failing with #142352, I don't see how it could have accepted c-int-width being a string...

@RalfJung
Copy link
Member

It might be that the field of wrong type just got ignored, which is exactly why Nora ported this to serde.

@WaffleLapkin
Copy link
Member Author

You are right indeed. The code in #142352:

                if let Some(s) = obj.remove(name).and_then(|b| b.as_u64()) {
                    base.$key_name = s as $int_ty;
                }

and_then(|b| b.as_u64()) makes it so non-integers are ignored.

@rust-log-analyzer

This comment has been minimized.

rust-timer added a commit that referenced this pull request Aug 4, 2025
Rollup merge of #144848 - Zalathar:ui-fulldeps, r=clubby789

For "stage 1" ui-fulldeps, use the stage 1 compiler to query target info

Testing ui-fulldeps in "stage 1" actually uses the stage 0 compiler, so that test programs can link against stage 1 rustc crates.

Unfortunately, using the stage 0 compiler causes problems when compiletest tries to obtain target information from the compiler, but the output format has changed since the last bootstrap beta bump.

We can work around this by also providing compiletest with a stage 1 compiler, and having it use that compiler to query for target information.

---

This fixes the stage 1 ui-fulldeps failure seen at #144443 (comment).
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Aug 4, 2025
For "stage 1" ui-fulldeps, use the stage 1 compiler to query target info

Testing ui-fulldeps in "stage 1" actually uses the stage 0 compiler, so that test programs can link against stage 1 rustc crates.

Unfortunately, using the stage 0 compiler causes problems when compiletest tries to obtain target information from the compiler, but the output format has changed since the last bootstrap beta bump.

We can work around this by also providing compiletest with a stage 1 compiler, and having it use that compiler to query for target information.

---

This fixes the stage 1 ui-fulldeps failure seen at rust-lang/rust#144443 (comment).
@WaffleLapkin WaffleLapkin force-pushed the integer-target-pointer-width branch from ffe4f0c to 1a5b2b0 Compare August 4, 2025 07:16
@WaffleLapkin WaffleLapkin force-pushed the integer-target-pointer-width branch from 1a5b2b0 to c4ca575 Compare August 4, 2025 09:11
@WaffleLapkin
Copy link
Member Author

WaffleLapkin commented Aug 4, 2025

@bors try jobs=x86_64-gnu-llvm-19-3

let's check if the failure is gone now that #144848 is merged

@rust-bors
Copy link

rust-bors bot commented Aug 4, 2025

⌛ Trying commit c4ca575 with merge f7c55e9

To cancel the try build, run the command @bors try cancel.

rust-bors bot added a commit that referenced this pull request Aug 4, 2025
…<try>

Make target pointer width in target json an integer

try-job: x86_64-gnu-llvm-19-3
@rust-bors
Copy link

rust-bors bot commented Aug 4, 2025

☀️ Try build successful (CI)
Build commit: f7c55e9 (f7c55e90b43e1f04d6484737deed9bc4d9584a16, parent: e1b9081e699065badfc1a9419ec9566e5c8615c4)

@WaffleLapkin
Copy link
Member Author

@bors r=Noratrieb

@bors
Copy link
Collaborator

bors commented Aug 4, 2025

📌 Commit c4ca575 has been approved by Noratrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 4, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 5, 2025
…r-width, r=Noratrieb

Make target pointer width in target json an integer

r? Noratrieb
cc `@RalfJung` (https://github.com/rust-lang/rust/pull/142352/files#r2230380120)
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 5, 2025
…r-width, r=Noratrieb

Make target pointer width in target json an integer

r? Noratrieb
cc ``@RalfJung`` (https://github.com/rust-lang/rust/pull/142352/files#r2230380120)
bors added a commit that referenced this pull request Aug 5, 2025
Rollup of 16 pull requests

Successful merges:

 - #143857 (Port #[macro_export] to the new attribute parsing infrastructure)
 - #144133 (Stabilize const TypeId::of)
 - #144369 (Upgrade semicolon_in_expressions_from_macros from warn to deny)
 - #144443 (Make target pointer width in target json an integer)
 - #144473 (Address libunwind.a inconsistency issues in the bootstrap program)
 - #144659 (bootstrap: refactor mingw dist and fix gnullvm)
 - #144682 (Stabilize `strict_overflow_ops`)
 - #144794 (Port `#[coroutine]` to the new attribute system)
 - #144835 (Anonymize binders in tail call sig)
 - #144836 (Change visibility of Args new function)
 - #144900 (Stabilize `unsigned_signed_diff` feature)
 - #144903 (Rename `begin_panic_handler` to `panic_handler`)
 - #144910 (Add regression tests for seemingly fixed issues)
 - #144926 (Correct the use of `must_use` on btree::IterMut)
 - #144928 (Drop `rust-version` from `rustc_thread_pool`)
 - #144931 ([win][arm64ec] Fix msvc-wholearchive for Arm64EC)

Failed merges:

 - #144914 (Add support for `ty::Instance` path shortening in diagnostics)

r? `@ghost`
`@rustbot` modify labels: rollup
@Kobzol
Copy link
Member

Kobzol commented Aug 5, 2025

Most likely failed in #144942 (comment).

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 5, 2025
@Zalathar

This comment was marked as outdated.

@Zalathar
Copy link
Contributor

Zalathar commented Aug 5, 2025

Ah, the problem seems to be this cargo test including a hardcoded target.json file with the older syntax: rust-lang/cargo#15026

@WaffleLapkin WaffleLapkin force-pushed the integer-target-pointer-width branch from c4ca575 to 51134eb Compare August 5, 2025 13:58
@rustbot rustbot added the A-compiler-builtins Area: compiler-builtins (https://github.com/rust-lang/compiler-builtins) label Aug 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 5, 2025

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

compiler-builtins is developed in its own repository. If possible, consider making this change to rust-lang/compiler-builtins instead.

cc @tgross35

@WaffleLapkin
Copy link
Member Author

I ran rg '"target-pointer-width": "' and fixed all the occurrences, maybe now it'll be fine :')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiler-builtins Area: compiler-builtins (https://github.com/rust-lang/compiler-builtins) A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants