-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix representation when printing abstract consts #119212
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
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jackh726 (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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 like this a lot
Please squash this into one commit, then I can approve it. @rustbot author |
c5e7a60
to
ac8c8d2
Compare
Thanks! Just squashed them into one commit. I also want to look into formatting the statements with parentheses based on the expr tree when I get the chance, as something like |
Ah, I had totally overlooked that. Thanks for pointing it out. I... actually don't think we should be printing out wrong expressions, and we should fix this in this PR. You should be able to get a precedence from a @rustbot author |
If you don't want to do this, let's just always parenthesize inner expressions, and I can implement a better heuristic for this. |
ac8c8d2
to
143e260
Compare
No worries, this new commit should take precedence into account when adding parenthesis. I didn't bother with fixity as the unop kind does not have an equivalent Having a fn for precedence in |
r? compiler-errors |
@w-utter any updates on this? thanks |
☔ The latest upstream changes (presumably #122150) made this pull request unmergeable. Please resolve the merge conflicts. |
I completely forgot about this after holiday break 😅 It's finished as far as I remember, I was just waiting on approval Hopefully I can guess what the unconstrained generic const is first try and it can get merged as I don't have the environment to test it set up anymore |
There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged. You can start a rebase with the following commands:
The following commits are merge commits: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@w-utter this still has a merge commit in the list of commits. You have to unmerge it or reset that commit using an interactive rebase. |
f888724
to
63fb0de
Compare
63fb0de
to
e247579
Compare
This comment has been minimized.
This comment has been minimized.
e247579
to
78e1869
Compare
This comment has been minimized.
This comment has been minimized.
78e1869
to
7c4b07d
Compare
That took a lot longer than I'd like to admit but we should be all good to go now 😁 |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (8579a18): 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)This benchmark run did not return any relevant results for this metric. 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: 669.426s -> 669.396s (-0.00%) |
Previously, when printing a const generic expr, it would only display it as
{{const expr}}
. This allows for a more legible representation when printing these out.I also zipped the types with their constants for abstract consts that contain function calls when using type annotations, eg:
foo(S: usize, true: bool) -> usize
insteaad offoo(S, true): fn(usize, bool) -> usize
for conciseness.