Skip to content

Conversation

@tshepang
Copy link
Member

@tshepang tshepang commented Oct 3, 2025

Also, add Reference ids

@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 Oct 3, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 3, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
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

@petrochenkov
Copy link
Contributor

  • I don't see any existing uses of //@ edition ranges in the test suite.
    When is it supposed to be used? Why in this test specifically?
  • When are the //@ reference directives supposed to be used?
    If ~all tests will have them then it's fine, otherwise it's not clear why this test should have them. It's not directly related to some reference clause, it's not even useful in general, and is mostly an example of reddit-oriented testing.

@petrochenkov petrochenkov 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2025
@tshepang
Copy link
Member Author

tshepang commented Oct 3, 2025

@petrochenkov
Copy link
Contributor

I've seen #146166, but it doesn't answer the question.
I'd expect the edition ranges to be (semi) automatically mass-annotated as a part of #145364.

For addition of the reference annotations I'd also expect to be done more systematically, similarly to #132376, but that's probably less likely to happen.

@petrochenkov
Copy link
Contributor

The change itself is harmless, but it's sort of like manually formatting code one function at a time instead of rustfmting the whole codebase, I don't want to encourage this.

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 3, 2025
@tshepang
Copy link
Member Author

tshepang commented Oct 3, 2025

  • lowered edition to 2018 because that test can run on that edition
  • made it a range because the test can also run on later editions
  • I do not expect an automatic edition mass-annotation to lower the edition of this test automatically
  • I added a reference id since I was touching it anyway... doing it more systematically feels much out of scope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants