-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Implement range support in //@ edition
#146166
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
Conversation
This comment has been minimized.
This comment has been minimized.
a19047a
to
714f2ca
Compare
r? fmease |
Some changes occurred in src/tools/compiletest cc @jieyouxu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial pass. I still need to think about the implication of this a bit.
@jieyouxu this is ready for another review I think 🎉 |
RangeFrom(Edition), | ||
/// Half-open range: `[lower_bound, upper_bound)` | ||
Range { | ||
lower_bound: Edition, | ||
upper_bound: Edition, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These could trivially be collapsed into Range { lower: Edition, upper: Option<Edition> }
+ using e.g., .map_or(true,
inside edition_to_test
but if you both find the current version more legible, I'm okay with it and won't block it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Once https://github.com/rust-lang/rust/pull/146166/files#r2356501671 is addressed (unless it's too painful), this looks good to go to me :)
Ofc, I'll be waiting for Jieyou Xu's final ACK. Then r=fmease,jieyouxu (maybe after squash, idc)
3dcbda0
to
6631c8e
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
@bors r=fmease,jieyouxu |
Implement range support in `//@ edition` First step to solve #145364
💔 Test failed - checks-actions |
Looking at the rich GHA logs of the failed job I see an abrupt end at line 10655 simply containing I tried to view the raw logs but that failed with Failed to generate URL to download logs.; I tried to download log archive but the download failed after several MBs transmitted. This gotta be spurious! @bors retry |
Rollup of 6 pull requests Successful merges: - #141839 (make rust-analyzer use a dedicated build directory) - #146166 (Implement range support in `//@ edition`) - #147259 (cg_llvm: Use helper methods for all calls to `LLVMMDNodeInContext2`) - #147263 (Disable triagebot auto stable-regression compiler backport nominations pending redesign) - #147268 (add arm-maintainers to various targets) - #147270 (Move doc_cfg-specific code into `cfg.rs`) r? `@ghost` `@rustbot` modify labels: rollup
Bors hasn't noticed that this was merged. @bors r- |
Rollup of 6 pull requests Successful merges: - rust-lang/rust#141839 (make rust-analyzer use a dedicated build directory) - rust-lang/rust#146166 (Implement range support in `//@ edition`) - rust-lang/rust#147259 (cg_llvm: Use helper methods for all calls to `LLVMMDNodeInContext2`) - rust-lang/rust#147263 (Disable triagebot auto stable-regression compiler backport nominations pending redesign) - rust-lang/rust#147268 (add arm-maintainers to various targets) - rust-lang/rust#147270 (Move doc_cfg-specific code into `cfg.rs`) r? `@ghost` `@rustbot` modify labels: rollup
First step to solve #145364