Fix conversion from Miri Value to ConstValue #50710

Zoxc · 2018-05-13T15:03:29Z

This fixes an error compiling the immeta 0.3.6 crate. #50707 may be fixed too.

oli-obk · 2018-05-13T15:08:44Z

src/test/run-pass/match-larger-const.rs

@@ -0,0 +1,12 @@
+
+#[derive(Eq, PartialEq)]


this test works on the nightly playpen: http://play.integer32.com/?gist=550e66e34e4ba58a8e716f800d94520f&version=nightly&mode=debug

The playpen is probably not on the latest nightly.

ah indeed. sorry

oli-obk · 2018-05-13T15:09:54Z

src/librustc_mir/interpret/const_eval.rs

-                               .get_alloc(ptr.alloc_id)
-                               .expect("miri allocation never successfully created");
-                assert_eq!(align, alloc.align);
+                let alloc = ecx.memory.get(ptr.alloc_id)?;


doesn't this change simply turn a panic into an error? How does that fix the regression?

The alloc ID we are trying to read is stored in the EvalContext and isn't in the global table.

ah. makes sense. Is there any reason reading this should ever fail? If so, then this code is fine. Otherwise it should probably have the panic, right?

This should really recurse into allocations like mark_static_initialized and could replace it by calling value_to_const_value at the end of const_eval.

ah. makes sense. Is there any reason reading this should ever fail?

I don't think this can fail. Value::ByRef here must always point to a valid AllocId right?

Might point to an integer address in general, but not as the result of const_eval

value_to_const_value is really just converting between different representation of Rust values, so it shouldn't fail. I've changed it to use unwrap so it will panic if it happens to fail.

rust-highfive · 2018-05-13T15:21:53Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.


[00:05:37] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:05:37] tidy error: /checkout/src/test/run-pass/match-larger-const.rs: incorrect license
[00:05:37] tidy error: /checkout/src/test/run-pass/match-larger-const.rs: missing trailing newline
[00:05:38] some tidy checks failed
[00:05:38] 
[00:05:38] 
[00:05:38] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:05:38] 
[00:05:38] 
[00:05:38] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:38] Build completed unsuccessfully in 0:02:12
[00:05:38] Build completed unsuccessfully in 0:02:12
[00:05:38] make: *** [tidy] Error 1
[00:05:38] Makefile:79: recipe for target 'tidy' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:03afe735
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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 @TimNN. (Feature Requests)

Zoxc · 2018-05-13T16:42:48Z

src/librustc_mir/interpret/const_eval.rs

    ty: Ty<'tcx>,
 ) -> &'tcx ty::Const<'tcx> {
-    let layout = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)).unwrap();
+    // Convert to ByVal or ByValPair if possible


I've moved the code to convert to ByVal and ByValPair in here.

so https://github.com/rust-lang/rust/pull/50710/files#diff-2a7855a4ad9a8ae6de5b6c25045a41d7R213 is obsolete now?

Possibly. It is obsolete when it's called from const_eval at least.

oli-obk · 2018-05-13T16:51:26Z

src/librustc_mir/interpret/const_eval.rs

-                               .get_alloc(ptr.alloc_id)
-                               .expect("miri allocation never successfully created");
-                assert_eq!(align, alloc.align);
+                let alloc = ecx.memory.get(ptr.alloc_id).unwrap();


please use expect so we can find it quickly if it fails (even if the issue reported has no backtrace)

I've changed this to report any miri errors then panic.

oli-obk · 2018-05-13T17:51:46Z

@bors r+

bors · 2018-05-13T17:51:47Z

📌 Commit 41a032d has been approved by oli-obk

kennytm · 2018-05-16T16:02:48Z

@bors p=7

https://botbot.me/mozilla/rustc/2018-05-16/?msg=100127264&page=4

bors · 2018-05-16T21:01:38Z

⌛ Testing commit 41a032d with merge 2f2a11d...

@oli-obk

Fix conversion from Miri Value to ConstValue This fixes an error compiling the `immeta` 0.3.6 crate. #50707 may be fixed too. r? @oli-obk

bors · 2018-05-16T23:46:16Z

☀️ Test successful - status-appveyor, status-travis
Approved by: oli-obk
Pushing 2f2a11d to master...

rust-highfive assigned oli-obk May 13, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2018

oli-obk reviewed May 13, 2018

View reviewed changes

Zoxc force-pushed the value_to_constvalue branch 3 times, most recently from 3d50de3 to 81915af Compare May 13, 2018 16:41

Zoxc commented May 13, 2018

View reviewed changes

oli-obk reviewed May 13, 2018

View reviewed changes

Fix conversion from Miri Value to ConstValue

41a032d

Zoxc force-pushed the value_to_constvalue branch from 81915af to 41a032d Compare May 13, 2018 17:04

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 13, 2018

bors added a commit that referenced this pull request May 16, 2018

Auto merge of #50710 - Zoxc:value_to_constvalue, r=oli-obk

2f2a11d

Fix conversion from Miri Value to ConstValue This fixes an error compiling the `immeta` 0.3.6 crate. #50707 may be fixed too. r? @oli-obk

bors merged commit 41a032d into rust-lang:master May 16, 2018

This was referenced May 16, 2018

Misc changes related to Miri allocations #50520

Merged

Ensure that statics are always ByRef #50690

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix conversion from Miri Value to ConstValue #50710

Fix conversion from Miri Value to ConstValue #50710

Zoxc commented May 13, 2018

oli-obk May 13, 2018

Zoxc May 13, 2018

oli-obk May 13, 2018

oli-obk May 13, 2018

Zoxc May 13, 2018

oli-obk May 13, 2018

Zoxc May 13, 2018

Zoxc May 13, 2018

oli-obk May 13, 2018

Zoxc May 13, 2018

rust-highfive commented May 13, 2018

Zoxc May 13, 2018

oli-obk May 13, 2018

Zoxc May 13, 2018

oli-obk May 13, 2018

Zoxc May 13, 2018

oli-obk commented May 13, 2018

bors commented May 13, 2018

kennytm commented May 16, 2018

bors commented May 16, 2018

bors commented May 16, 2018

Fix conversion from Miri Value to ConstValue #50710

Fix conversion from Miri Value to ConstValue #50710

Conversation

Zoxc commented May 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rust-highfive commented May 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oli-obk commented May 13, 2018

bors commented May 13, 2018

kennytm commented May 16, 2018

bors commented May 16, 2018

bors commented May 16, 2018