Skip to content

Explicitly annotate edition for unpretty=expanded and unpretty=hir tests #139904

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 17, 2025
Merged

Explicitly annotate edition for unpretty=expanded and unpretty=hir tests #139904

merged 1 commit into from
Apr 17, 2025

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Apr 16, 2025

These emit prelude imports which means they are always edition dependent and so running them with a different --edition will fail.

@Veykril
Explicitly annotate edition for unpretty=expanded and `unpretty=hir…
20ab952 
…` tests

These emit prelude imports which means they are always edition dependent
@rustbot
Copy link
Collaborator

rustbot commented Apr 16, 2025

r? @nnethercote

rustbot has assigned @nnethercote.
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 Apr 16, 2025
@nnethercote
Copy link
Contributor

Is this to support rust-lang/compiler-team#861 ? That MCP hasn't been accepted yet and currently has some registered concerns, so is this PR premature?

@Veykril
Copy link
Member Author

Veykril commented Apr 16, 2025 •
edited
Loading

Yes and no, the status quo is that these tests (and around 500 more) fail if you run x.py test --edition EDITION with any edition other than 2015. The MCP does raise fixing these tests as part of it but the main part of the MCP is the new @edition syntax (allowing ranges) as well as things relating to the "default edition".

If fixing tests is blocked on the MCP then fair enough, it'll have to wait.

@nnethercote
Copy link
Contributor

fail if you run x.py test --edition EDITION with any edition other than 2015

Is that currently an interesting use case? Does anyone do that? Sorry for the questions, I'm just trying to get my head around the change.

@Veykril
Copy link
Member Author

Veykril commented Apr 16, 2025 •
edited
Loading

Sorry for the questions, I'm just trying to get my head around the change.

No problem. I imagine we are the first trying to run the entire test suite with a differing edition from the default (but until recently, it was also broken when passed the default, that is 2015 edition, explicitly #139578).

So right, now, the --edition flag is broken for non 2015 editions either way. It is also clear that just throwing fixes for all 500+ tests that currently break isn't something we should pursue until the status of the MCP is resolved but I believe the ones fixed here are sensible in that their output specifically mentions the edition they run against, and so marking them explicit seems like the proper way to fix them (where as for other tests it might be less clear what the "fix" for them is going to be).

@jieyouxu
Copy link
Member

I don't think we should add explicit //@ edition: 2015 directives yet (which is the default edition for ui tests), I'd like to see some of the edition discussions on the MCP settled down first.

@nnethercote
Copy link
Contributor

The PR seems fine in terms of the changes achieving what they set out to do. But @jieyouxu is much more involved with test stuff than I am so I will defer to their opinion about waiting here.

@jieyouxu
Copy link
Member

Actually, I looked at the test diffs more closely, and these do need edition 2015 which is not like other tests that don't really "care" about editions.

@jieyouxu
Copy link
Member

So in that sense, this looks good to me as well and doesn't warrant blocking on the overall MCP. Thanks!

@bors r=nnethercote,jieyouxu rollup

@bors
Copy link
Collaborator

bors commented Apr 17, 2025

📌 Commit 20ab952 has been approved by nnethercote,jieyouxu

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 Apr 17, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 17, 2025
Merged
@jieyouxu jieyouxu self-assigned this Apr 17, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2025
@bors
Auto merge of rust-lang#139949 - matthiaskrgr:rollup-pxc5tsx, r=matth…
883f9f7 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#138632 (Stabilize `cfg_boolean_literals`)
 - rust-lang#139416 (unstable book; document `macro_metavar_expr_concat`)
 - rust-lang#139782 (Consistent with treating Ctor Call as Struct in liveness analysis)
 - rust-lang#139885 (document RUSTC_BOOTSTRAP, RUSTC_OVERRIDE_VERSION_STRING, and -Z allow-features in the unstable book)
 - rust-lang#139904 (Explicitly annotate edition for `unpretty=expanded` and `unpretty=hir` tests)
 - rust-lang#139932 (transmutability: Refactor tests for simplicity)
 - rust-lang#139944 (Move eager translation to a method on Diag)
 - rust-lang#139948 (git: ignore `60600a6fa403216bfd66e04f948b1822f6450af7` for blame purposes)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4a83f43 into rust-lang:master Apr 17, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 17, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2025
@rust-timer
Unrolled build for rust-lang#139904
1f11e54 
Rollup merge of rust-lang#139904 - ferrocene:lw-wkumpwrytvtp, r=nnethercote,jieyouxu

Explicitly annotate edition for `unpretty=expanded` and `unpretty=hir` tests

These emit prelude imports which means they are always edition dependent and so running them with a different `--edition` will fail.
nnethercote
nnethercote reviewed Apr 17, 2025
View reviewed changes
diff
Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops, this shouldn't have been committed!

Copy link
Contributor

Choose a reason for hiding this comment

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

I added a commit to #139897 to remove it.

(I noticed this because I regularly redirect the output of git diff and similar commands to a file called diff.)

Copy link
Member

Choose a reason for hiding this comment

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

Oh oops I didn't notice this stray file

nnethercote added a commit to nnethercote/rust that referenced this pull request Apr 17, 2025
@nnethercote
Remove diff file.
642cfba 
It was accidentally committed in rust-lang#139904.
@nnethercote nnethercote mentioned this pull request Apr 17, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nnethercote nnethercote nnethercote left review comments

@jieyouxu jieyouxu jieyouxu left review comments

Assignees

@nnethercote nnethercote

@jieyouxu jieyouxu

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
Milestone
1.88.0
Development

Successfully merging this pull request may close these issues.
5 participants
@Veykril @rustbot @nnethercote @jieyouxu @bors