-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Fixes and cleanups #58000
Fixes and cleanups #58000
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
// State of a local variable | ||
/// State of a local variable including a memoized layout | ||
#[derive(Clone, PartialEq, Eq)] | ||
pub struct LocalValue<'tcx, Tag=(), Id=AllocId> { |
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.
Nit: I think swapping the names LocalValue
and LocalState
is nicer. The layout
stuff looks much more state-y than value-y to me.
type Item = LocalValue<(), AllocIdSnapshot<'a>>; | ||
|
||
fn snapshot(&self, ctx: &'a Ctx) -> Self::Item { | ||
self.state.snapshot(ctx) |
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.
We otherwise follow the pattern to use LocalState { state, layout: _ } = self
to make sure we revisit this when new fields get added.
impl_snapshot_for!
doesn't work?
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.
nope, it's a very simple macro with no support for lifetimes or so
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.
Done and opened rust-lang/rust-clippy#3724 to make sure we take care of this issue once and for all at some point
@bors r+ |
📌 Commit 8c26c59 has been approved by |
Fixes and cleanups Address the points raised in https://github.com/rust-lang/rust/pull/57677/files by @eddyb and @RalfJung
Fixes and cleanups Address the points raised in https://github.com/rust-lang/rust/pull/57677/files by @eddyb and @RalfJung
Fixes and cleanups Address the points raised in https://github.com/rust-lang/rust/pull/57677/files by @eddyb and @RalfJung
Rollup of 12 pull requests Successful merges: - #57008 (suggest `|` when `,` founds in invalid match value) - #57106 (Mark str::trim.* functions as #[must_use].) - #57920 (use `SOURCE_DATE_EPOCH` for man page time if set) - #57934 (Introduce into_raw_non_null on Rc and Arc) - #57971 (SGX target: improve panic & exit handling) - #57980 (Add the edition guide to the bookshelf) - #57984 (Improve bug message in check_ty) - #57999 (Add MOVBE x86 CPU feature) - #58000 (Fixes and cleanups) - #58005 (update docs for fix_start/end_matches) - #58007 (Don't panic when accessing enum variant ctor using `Self` in match) - #58008 (Pass correct arguments to places_conflict) Failed merges: r? @ghost
@bors retry |
@bors r- retry |
Address the points raised in https://github.com/rust-lang/rust/pull/57677/files by @eddyb and @RalfJung