-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Minor improvements to rustc_ast_passes
#117204
Conversation
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
crate::fluent_generated::ast_passes_forbidden_non_lifetime_param, | ||
) | ||
.emit(); | ||
if !non_lt_param_spans.is_empty() { |
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.
What this will do is emit a feature error for every single span. What I want is for it to be a single feature error with multiple primary spans.
I prefer if this is not changed, or it is fixed in the right way.
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.
IDK why there isn't a test for this. Here's one:
fn test() -> for<T, S> fn(T, S) { todo!() }
error[E0658]: only lifetime parameters can be used in this context
--> src/lib.rs:1:18
|
1 | fn test() -> for<T, S> fn(T, S) { todo!() }
| ^ ^
|
= note: see issue #108185 <https://github.com/rust-lang/rust/issues/108185> for more information
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.
👍
I have updated the code, removing the invalid commit, and adding three new commits: another small refactor, an alternative fix for the FIXME, and a test update. Hopefully that's closer to what is necessary :) Those macros still annoy me, but I can't see a better way to do things without increasing the amount of boilerplate.
239e59b
to
8d9b115
Compare
The debug probably isn't useful, and assigning all the `$foo` metavariables to `foo` variables is verbose and weird. Also, `$x:expr` usually doesn't have a space after the `:`.
Note that this adds the `span.allows_unstable` checking that this case previously lacked.
8d9b115
to
5b391b0
Compare
I fixed the conflicts. |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (62270fb): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 639.997s -> 639.068s (-0.15%) |
82: Automated pull from upstream `master` r=tshepang a=github-actions[bot] This PR pulls the following changes from the upstream repository: * rust-lang/rust#117313 * rust-lang/rust#117131 * rust-lang/rust#117134 * rust-lang/rust#117471 * rust-lang/rust#117521 * rust-lang/rust#117513 * rust-lang/rust#117512 * rust-lang/rust#117509 * rust-lang/rust#117495 * rust-lang/rust#117394 * rust-lang/rust#117466 * rust-lang/rust#117204 * rust-lang/rust#117386 * rust-lang/rust#117506 Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com> Co-authored-by: roblabla <unfiltered@roblab.la> Co-authored-by: Michael Goulet <michael@errs.io> Co-authored-by: massivebird <gdrakemail@gmail.com> Co-authored-by: bors <bors@rust-lang.org> Co-authored-by: Zalathar <Zalathar@users.noreply.github.com> Co-authored-by: lcnr <rust@lcnr.de> Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com> Co-authored-by: Matthias Krüger <matthias.krueger@famsik.de>
Some improvements I found while looking at this code.
r? @compiler-errors