Skip to content
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

Add the new description field to Target::to_json, and add descriptions for some MSVC targets #122157

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

dpaoliello
Copy link
Contributor

The original PR to add a description field to Target (#121905) didn't add the field to Target::to_json, which meant that the check_consistency testwould fail if you tried to set a description as it wouldn't survive round-tripping via JSON: https://github.com/rust-lang/rust/actions/runs/8180997936/job/22370052535#step:27:4967

This change adds the field to Target::to_json, and sets some descriptions to verify that it works correctly.

@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2024

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 7, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2024

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

@dpaoliello
Copy link
Contributor Author

r? @Nilstrieb

@rustbot rustbot assigned Noratrieb and unassigned TaKO8Ki Mar 7, 2024
@Noratrieb
Copy link
Member

Having 64-bit MSVC next to ARM64 MSVC looks pretty silly, but whatever, not your fault
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 8, 2024

📌 Commit d6b597b has been approved by Nilstrieb

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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 8, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#121201 (align_offset, align_to: no longer allow implementations to spuriously fail to align)
 - rust-lang#122076 (Tweak the way we protect in-place function arguments in interpreters)
 - rust-lang#122100 (Better comment for implicit captures in RPITIT)
 - rust-lang#122157 (Add the new description field to Target::to_json, and add descriptions for some MSVC targets)
 - rust-lang#122164 (Fix misaligned loads when loading UEFI arg pointers)
 - rust-lang#122171 (Add some new solver tests)
 - rust-lang#122172 (Don't ICE if we collect no RPITITs unless there are no unification errors)
 - rust-lang#122197 (inspect formatter: add braces)
 - rust-lang#122198 (Remove handling for previously dropped LLVM version)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b9a3952 into rust-lang:master Mar 8, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2024
Rollup merge of rust-lang#122157 - dpaoliello:targetdesc, r=Nilstrieb

Add the new description field to Target::to_json, and add descriptions for some MSVC targets

The original PR to add a `description` field to `Target` (<rust-lang#121905>) didn't add the field to `Target::to_json`, which meant that the `check_consistency` testwould fail if you tried to set a description as it wouldn't survive round-tripping via JSON: https://github.com/rust-lang/rust/actions/runs/8180997936/job/22370052535#step:27:4967

This change adds the field to `Target::to_json`, and sets some descriptions to verify that it works correctly.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2024
Add metadata to targets

follow up to rust-lang#121905 and rust-lang#122157

This adds four pieces of metadata to every target:
- description
- tier
- host tools
- std

This information is currently scattered across target docs and both
- not machine readable, making validation harder
- sometimes subtly encoding by the table it's in, causing mistakes and making it harder to review changes to the properties

By putting it in the compiler, we improve this. Later, we will use this canonical information to generate target documentation from it.

I used find-replace for all the `description: None`.

One thing I'm not sure about is the behavior for the JSON. It doesn't really make sense that custom targets supply this information, especially the tier. But for the roundtrip tests, we do need to print and parse it. Maybe emit a warning when a custom target provides the metadata key? Either way, I don't think that's important right now, this PR should get merged ASAP or it will conflict all over the place.

r? davidtwco
@dpaoliello dpaoliello deleted the targetdesc branch March 11, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

5 participants