-
Notifications
You must be signed in to change notification settings - Fork 13k
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
BindingAnnotation
refactor
#101241
BindingAnnotation
refactor
#101241
Conversation
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
a597649
to
aaf9b68
Compare
This comment has been minimized.
This comment has been minimized.
aaf9b68
to
097ca4d
Compare
☔ The latest upstream changes (presumably #101249) made this pull request unmergeable. Please resolve the merge conflicts. |
097ca4d
to
9ea82d5
Compare
@bors r+ |
⌛ Testing commit 9ea82d5 with merge b52a3be1828c81008d625765dc480609524da515... |
💔 Test failed - checks-actions |
Looks quite spurious. @bors retry |
The job Click to see the possible cause of the failure (guessed by this bot)
|
☀️ Test successful - checks-actions |
Finished benchmarking commit (6c358c6): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
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.
CyclesResultsThis 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.
Footnotes |
…ons, r=cjgillot `BindingAnnotation` refactor * `ast::BindingMode` is deleted and replaced with `hir::BindingAnnotation` (which is moved to `ast`) * `BindingAnnotation` is changed from an enum to a tuple struct e.g. `BindingAnnotation(ByRef::No, Mutability::Mut)` * Associated constants added for convenience `BindingAnnotation::{NONE, REF, MUT, REF_MUT}` One goal is to make it more clear that `BindingAnnotation` merely represents syntax `ref mut` and not the actual binding mode. This was especially confusing since we had `ast::BindingMode`->`hir::BindingAnnotation`->`thir::BindingMode`. I wish there were more symmetry between `ByRef` and `Mutability` (variant) naming (maybe `Mutable::Yes`?), and I also don't love how long the name `BindingAnnotation` is, but this seems like the best compromise. Ideas welcome.
… have been replaced by associated constants <rust-lang/rust#101241>
… have been replaced by associated constants <rust-lang/rust#101241>
ast::BindingMode
is deleted and replaced withhir::BindingAnnotation
(which is moved toast
)BindingAnnotation
is changed from an enum to a tuple struct e.g.BindingAnnotation(ByRef::No, Mutability::Mut)
BindingAnnotation::{NONE, REF, MUT, REF_MUT}
One goal is to make it more clear that
BindingAnnotation
merely represents syntaxref mut
and not the actual binding mode. This was especially confusing since we hadast::BindingMode
->hir::BindingAnnotation
->thir::BindingMode
.I wish there were more symmetry between
ByRef
andMutability
(variant) naming (maybeMutable::Yes
?), and I also don't love how long the nameBindingAnnotation
is, but this seems like the best compromise. Ideas welcome.