Various const eval and pattern matching ICE fixes by oli-obk · Pull Request #67192 · rust-lang/rust

oli-obk · 2019-12-10T11:58:10Z

This PR does not change existing behaviour anymore and just fixes a bunch of ICEs reachable from user code (sometimes even on stable via obscure union transmutes).

oli-obk · 2019-12-10T11:58:36Z

cc @varkor for the match code changes

RalfJung · 2019-12-10T17:49:47Z

src/librustc_mir/interpret/eval_context.rs

This chain of FIXMEs is getting some weird kind of joke.^^

I think it referred to the "lazy" vs "evaluated" AllocId for statics?

The last two are gone in my PR 😄

src/librustc_mir/interpret/eval_context.rs

src/librustc_mir/interpret/operand.rs

src/librustc_mir/interpret/place.rs

RalfJung · 2019-12-10T17:56:19Z

I don't understand the motivation for this. This changes one of many allocation methods to avoid ZST, but we may still get empty allocations elsewhere. (At least, there's no assertion that I could see in Memory or so that would prevent this.) You are linking to codegen but I have no idea what that code is telling me... something is special-casing ZST. Okay, so... what does that have to do with this PR?^^ The code in codegen creates an "undef immediate", while CTFE uses an integer ptr, so if this PR is about making codegen and CTFE more consistent then I don't see it.

src/librustc_mir/interpret/place.rs

oli-obk · 2019-12-10T19:43:56Z

I don't understand the motivation for this. This changes one of many allocation methods to avoid ZST, but we may still get empty allocations elsewhere.

I could set it up so ZST Stack allocations can never actually have alloc ids. That would fold all of this logic down to the allocate function instead of the piecemeal spread around the interpreter.

oli-obk · 2019-12-13T11:28:34Z

I tried to keep every change as independent of all the others as possible and make a commit for each step. Some of these could be pulled out into a separate PR without changing behaviour if you want.

src/test/ui/consts/issue-52432.stderr

bors · 2019-12-13T22:58:55Z

☔ The latest upstream changes (presumably #67284) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk · 2019-12-13T23:21:11Z

This PR has now turned around 100% and is not changing behaviour wrt zst addresses anymore

oli-obk · 2019-12-20T11:38:01Z

I believe I have addressed all review comments

@oli-obk

Promote references to constants instead of statics This sits on top of @oli-obk's #67192. When that's merged I can rebase and make it a final PR. r? @oli-obk

oli-obk · 2019-12-21T09:56:27Z

@bors try

bors · 2019-12-21T09:56:37Z

⌛ Trying commit d2ad810 with merge e45d134659ec3c8628d74e3ddb5c9f0d1df24223...

oli-obk · 2019-12-26T22:09:21Z

rebased over rustfmt and const_eval.rs split PRs

@bors r=RalfJung,varkor

bors · 2019-12-26T22:09:22Z

📌 Commit f65a91e has been approved by RalfJung,varkor

@RalfJung

…varkor Various const eval and pattern matching ICE fixes r? @RalfJung cc @spastorino This PR does not change existing behaviour anymore and just fixes a bunch of ICEs reachable from user code (sometimes even on stable via obscure union transmutes).

@ghost

Rollup of 12 pull requests Successful merges: - #67112 (Refactor expression parsing thoroughly) - #67192 (Various const eval and pattern matching ICE fixes) - #67287 (typeck: note other end-point when checking range pats) - #67459 (prune ill-conceived BTreeMap iter_mut assertion and test its mutability) - #67576 (reuse `capacity` variable in slice::repeat) - #67602 (Use issue = "none" instead of "0" in intrinsics) - #67614 (Set callbacks globally) - #67617 (Remove `compiler_builtins_lib` documentation) - #67629 (Remove redundant link texts) - #67632 (Convert collapsed to shortcut reference links) - #67633 (Update .mailmap) - #67635 (Document safety of Path casting) Failed merges: r? @ghost

bors · 2019-12-27T07:39:03Z

⌛ Testing commit f65a91e with merge 8f5f8f9...

@RalfJung

Various const eval and pattern matching ICE fixes r? @RalfJung cc @spastorino This PR does not change existing behaviour anymore and just fixes a bunch of ICEs reachable from user code (sometimes even on stable via obscure union transmutes).

bors · 2019-12-27T10:58:50Z

☀️ Test successful - checks-azure
Approved by: RalfJung,varkor
Pushing 8f5f8f9 to master...

rust-highfive · 2019-12-27T11:00:09Z

📣 Toolstate changed by #67192!

Tested on commit 8f5f8f9.
Direct link to PR: #67192

💔 miri on windows: test-pass → build-fail (cc @oli-obk @eddyb @RalfJung, @rust-lang/infra).
💔 miri on linux: test-pass → build-fail (cc @oli-obk @eddyb @RalfJung, @rust-lang/infra).

@oli-obk

Tested on commit rust-lang/rust@8f5f8f9. Direct link to PR: <rust-lang/rust#67192> 💔 miri on windows: test-pass → build-fail (cc @oli-obk @eddyb @RalfJung, @rust-lang/infra). 💔 miri on linux: test-pass → build-fail (cc @oli-obk @eddyb @RalfJung, @rust-lang/infra).

Remove all uses of `to_ptr` Also only works on top of rust-lang/rust#67192

rust-highfive assigned RalfJung Dec 10, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 10, 2019

spastorino approved these changes Dec 10, 2019

View reviewed changes

RalfJung reviewed Dec 10, 2019

View reviewed changes

src/librustc_mir/interpret/eval_context.rs Outdated Show resolved Hide resolved

RalfJung reviewed Dec 10, 2019

View reviewed changes

src/librustc_mir/interpret/operand.rs Outdated Show resolved Hide resolved

RalfJung reviewed Dec 10, 2019

View reviewed changes

src/librustc_mir/interpret/place.rs Outdated Show resolved Hide resolved

RalfJung reviewed Dec 10, 2019

View reviewed changes

src/librustc_mir/interpret/place.rs Outdated Show resolved Hide resolved

spastorino mentioned this pull request Dec 10, 2019

Promote references to constants instead of statics #67000

Merged

bjorn3 reviewed Dec 13, 2019

View reviewed changes

src/test/ui/consts/issue-52432.stderr Outdated Show resolved Hide resolved

oli-obk mentioned this pull request Dec 13, 2019

Remove all uses of to_ptr rust-lang/miri#1114

Merged

oli-obk force-pushed the const_zst_addr branch from d9aacf2 to 21ea367 Compare December 13, 2019 23:19

oli-obk changed the title ~~Don't generate AllocIds for ZST allocations~~ Various const eval and pattern matching ICE fixes Dec 13, 2019

oli-obk force-pushed the const_zst_addr branch from 21ea367 to b75a2a0 Compare December 14, 2019 08:13

This comment has been minimized.

Sign in to view

oli-obk force-pushed the const_zst_addr branch from 81fbca4 to c86fe4a Compare December 19, 2019 17:30

This comment has been minimized.

Sign in to view

oli-obk force-pushed the const_zst_addr branch from fe01f9b to d2ad810 Compare December 20, 2019 23:00

oli-obk mentioned this pull request Dec 21, 2019

ICE matching on a non-[u8] const array #66501

Closed

oli-obk added 8 commits December 26, 2019 22:50

Explain why const_eval is ok here

9520551

Early abort instead of building up zero sized values

1acbf4b

Add a const_eval helper to InterpCx

20c1b3f

Documentation nit

1531c39

Reintroduce the recursion comment

b476344

Use the targetted const eval functions

aaffe12

Fix rebase fallout

12a4c2c

Make ui test bitwidth independent

f65a91e

oli-obk force-pushed the const_zst_addr branch from ad964e7 to f65a91e Compare December 26, 2019 22:08

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 26, 2019

Mark-Simulacrum mentioned this pull request Dec 26, 2019

Rollup of 12 pull requests #67648

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 27, 2019

bors merged commit f65a91e into rust-lang:master Dec 27, 2019

rust-highfive mentioned this pull request Dec 27, 2019

miri no longer builds after rust-lang/rust#67192 #67655

Closed

bors mentioned this pull request Dec 27, 2019

Reduce special treatment for zsts #67501

Merged

bors added a commit to rust-lang/miri that referenced this pull request Dec 27, 2019

Auto merge of #1114 - rust-lang:to_ptr_considered_harmful, r=oli-obk

34d8ec8

Remove all uses of `to_ptr` Also only works on top of rust-lang/rust#67192

This was referenced Dec 28, 2019

Size mismatch when const transmuting #61774

Closed

transmute const generic parameter #61455

Closed

oli-obk deleted the const_zst_addr branch March 16, 2021 12:13

Uh oh!

Conversation

oli-obk commented Dec 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oli-obk commented Dec 10, 2019

Uh oh!

RalfJung Dec 10, 2019

Choose a reason for hiding this comment

Uh oh!

spastorino Dec 23, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RalfJung commented Dec 10, 2019

Uh oh!

Uh oh!

oli-obk commented Dec 10, 2019

Uh oh!

oli-obk commented Dec 13, 2019

Uh oh!

Uh oh!

bors commented Dec 13, 2019

Uh oh!

oli-obk commented Dec 13, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

oli-obk commented Dec 20, 2019

Uh oh!

oli-obk commented Dec 21, 2019

Uh oh!

bors commented Dec 21, 2019

Uh oh!

oli-obk commented Dec 26, 2019

Uh oh!

bors commented Dec 26, 2019

Uh oh!

bors commented Dec 27, 2019

Uh oh!

bors commented Dec 27, 2019

Uh oh!

rust-highfive commented Dec 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

oli-obk commented Dec 10, 2019 •

edited

Loading