unit rvalue, use constant () instead of tuple
#70891
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
lcnr
force-pushed
the
replace-rvalue_aggregate
branch
from
28a879a
to
f0801f7
Compare
|
LGTM, but let's wait for #70721 first (since |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Centril
reviewed
Apr 7, 2020
lcnr
force-pushed
the
replace-rvalue_aggregate
branch
from
f0801f7
to
6a9ffda
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
eddyb
reviewed
Apr 9, 2020
eddyb
added
S-waiting-on-author
|
#70721 landed, you need to rebase and run |
lcnr
force-pushed
the
replace-rvalue_aggregate
branch
from
6a9ffda
to
aca4294
Compare
|
done, these new mir dumps are far longer than the previous ones though |
eddyb
reviewed
Apr 10, 2020
Comment on lines
-301
to
+307
| _0 = (); // bb0[160]: scope 0 at $DIR/address-of.rs:3:26: 37:2 | ||
| _0 = const (); // bb0[160]: scope 0 at $DIR/address-of.rs:3:26: 37:2 | ||
| // ty::Const | ||
| // + ty: () | ||
| // + val: Value(Scalar(<ZST>)) | ||
| // mir::Constant | ||
| // + span: $DIR/address-of.rs:3:26: 37:2 | ||
| // + literal: Const { ty: (), val: Value(Scalar(<ZST>)) } |
There was a problem hiding this comment.
@rust-lang/wg-mir-opt Should we prioritize removing these verbose comments? I'm not sure they add much value.
(There's the Span but that only makes sense when it differs the statement Span, which it doesn't here)
|
Blocking this on #70989, as we want to be able to tell on the PR itself that |
eddyb
added
S-blocked
eddyb
reviewed
Apr 11, 2020
| Rvalue::Use(Operand::Constant(box Constant { | ||
| span: source_info.span, | ||
| user_ty: None, | ||
| literal: ty::Const::zero_sized(tcx, tcx.types.unit), |
There was a problem hiding this comment.
FWIW, I'm coincidentally adding tcx.consts.unit to #70970.
|
☔ The latest upstream changes (presumably #70989) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@lcnr If you rebase, I think CI should now show the 32-bit mismatches. |
lcnr
force-pushed
the
replace-rvalue_aggregate
branch
from
aca4294
to
4714e20
Compare
|
done 🤷♂️ what now? I doubt that changing all tests to only run on 64bit is a good idea. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@lcnr You manually fix it based on errors on CI (hard) or you run the |
|
Ok, Ready to merge afaict. |
eddyb
approved these changes
Apr 14, 2020
|
@bors r+ Thanks! |
|
📌 Commit 647f810 has been approved by |
bors
added
S-waiting-on-bors
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 15, 2020
unit rvalue, use constant `()` instead of tuple fixes rust-lang#70886 r? @eddyb
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 15, 2020
Rollup of 4 pull requests Successful merges: - rust-lang#70891 (unit rvalue, use constant `()` instead of tuple) - rust-lang#71030 (rustc_target: Refactor target specifications related to Windows and UEFI) - rust-lang#71100 (Miri: expand frame hooks) - rust-lang#71116 (Entirely remove `DUMMY_HIR_ID`) Failed merges: r? @ghost
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #70886
r? @eddyb