Allow assignments in const contexts #56070

oli-obk · 2018-11-19T12:49:35Z

fixes #54098
fixes #51251
fixes #52613

rust-highfive · 2018-11-19T12:49:41Z

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

src/librustc_mir/interpret/memory.rs

petrochenkov · 2018-11-19T23:29:55Z

r? @RalfJung

src/librustc_mir/transform/qualify_consts.rs

src/test/ui/consts/const-eval/mod-static-with-const-fn.rs

src/librustc_mir/interpret/machine.rs

RalfJung · 2018-11-20T11:28:31Z

r=me with tidy happy:

[00:03:46] tidy error: /checkout/src/test/ui/consts/promote_const_let.rs: missing trailing newline
[00:03:47] some tidy checks failed

oli-obk · 2018-11-20T13:12:45Z

@bors r=RalfJung

bors · 2018-11-20T13:12:46Z

📌 Commit 37fcd5c has been approved by RalfJung

eddyb · 2018-11-21T06:00:34Z

src/librustc_mir/transform/qualify_consts.rs

+                    Place::Local(index) => break *index,
+                    // projections are transparent for assignments
+                    // we qualify the entire destination at once, even if just a field would have
+                    // stricter qualification


This doesn't seem correct - if you assign something with less qualifications to a field of a value with more, you turn those bits off.

eddyb · 2018-11-21T06:01:32Z

In the future, please cc me on qualify_consts changes.

@bors r-

oli-obk · 2018-11-21T07:40:27Z

src/librustc_mir/transform/qualify_consts.rs

+                    // projections are transparent for assignments
+                    // we qualify the entire destination at once, even if just a field would have
+                    // stricter qualification
+                    Place::Projection(proj) => dest = &proj.base,


Continuing with

the mindset when dealing with [const qualif] should be "how do I break this?"

Projections would include Deref, which isn't a problem because we can't have mutable references at all inside const contexts. Still something to think about.

Downcast also can't happen, because we'd need match for that.

Maybe we should just whitelist field projections for now.

The problem is something like this:

const FOO: &(Cell<usize>, bool) = &{ let mut foo = (Cell::new(0), false); foo.1 = true; // resets `qualif(foo)` to `qualif(true)` foo };

I suspect that will actually compile with this PR, although I'd like a confirmation.

We could add this as a testcase, but you might be able to come up with something simpler/more general.

This indeed compiles successfully with this PR

My suggestion is field (or projections in general) assignment should combine qualifications (i.e. |=) instead of replacing them.

I now reuse the existing projection checks for reading values. These checks replace the qualification with NOT_CONST, so there's no real use in combining I think?

RalfJung · 2018-11-21T08:10:33Z

In the future, please cc me on qualify_consts changes.

I'll happily hand them over to you. ;)

r? @eddyb

rust-highfive · 2018-11-21T09:59:35Z

The job x86_64-gnu-llvm-5.0 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.

travis_time:end:065e7ce8:start=1542793454701620808,finish=1542793512762261299,duration=58060640491
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---

[00:04:52] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:52] tidy error: /checkout/src/test/ui/consts/projection_qualif.rs: missing trailing newline
[00:04:52] tidy error: /checkout/src/test/ui/consts/partial_qualif.rs: missing trailing newline
[00:04:53] some tidy checks failed
[00:04:53] 
[00:04:53] 
[00:04:53] 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:04:53] 
[00:04:53] 
[00:04:53] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:53] Build completed unsuccessfully in 0:01:02
[00:04:53] Build completed unsuccessfully in 0:01:02
[00:04:53] Makefile:79: recipe for target 'tidy' failed
[00:04:53] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:2218e2ff
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Wed Nov 21 09:50:16 UTC 2018
---
travis_time:end:22c719fc:start=1542793817090665327,finish=1542793817095606984,duration=4941657
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0497bbba
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:00f12be8
travis_time:start:00f12be8
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0ef37a6f
$ dmesg | grep -i kill

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)

src/librustc_mir/transform/qualify_consts.rs

oli-obk · 2018-11-21T10:59:06Z

@bors r=eddyb

bors · 2018-11-21T10:59:07Z

📌 Commit 6db8c6c has been approved by eddyb

bors · 2018-11-26T08:17:46Z

⌛ Testing commit 6db8c6c with merge b51632e...

Allow assignments in const contexts fixes #54098 fixes #51251 fixes #52613

bors · 2018-11-26T11:18:17Z

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing b51632e to master...

Allow assignments in const contexts

bc543d7

rust-highfive assigned petrochenkov Nov 19, 2018

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

oli-obk commented Nov 19, 2018

View reviewed changes

src/librustc_mir/interpret/memory.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Also catch static mutation at evaluation time

59eff14

oli-obk force-pushed the const_let branch from 393684d to 59eff14 Compare November 19, 2018 15:46

This comment has been minimized.

Sign in to view

Update nll stderr files

7d5b5ff

rust-highfive assigned RalfJung and unassigned petrochenkov Nov 19, 2018

RalfJung reviewed Nov 20, 2018

View reviewed changes

src/librustc_mir/transform/qualify_consts.rs Show resolved Hide resolved

RalfJung reviewed Nov 20, 2018

View reviewed changes

src/test/ui/consts/const-eval/mod-static-with-const-fn.rs Show resolved Hide resolved

oli-obk added 3 commits November 20, 2018 11:11

Document runtime static mutation checks

f98235e

Explain missing error in test

6bcb0d6

Add sanity test for promotion and const_let

f70abe8

RalfJung reviewed Nov 20, 2018

View reviewed changes

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

This comment has been minimized.

Sign in to view

oli-obk added 2 commits November 20, 2018 12:25

Document qualify_consts more

7d96f2c

Trailing newline

3e081c7

Grammar nit

37fcd5c

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 Nov 20, 2018

eddyb reviewed Nov 21, 2018

View reviewed changes

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 21, 2018

oli-obk commented Nov 21, 2018

View reviewed changes

rust-highfive assigned eddyb and unassigned RalfJung Nov 21, 2018

oli-obk added 2 commits November 21, 2018 10:37

Properly assign to aggregate fields

301ce8b

Ensure assignments don't allow skipping projection checks

3c290a5

Tidy

42a3f73

eddyb reviewed Nov 21, 2018

View reviewed changes

src/librustc_mir/transform/qualify_consts.rs Show resolved Hide resolved

oli-obk added 3 commits November 21, 2018 11:47

Fix a comment

e05b61c

Add regression test for overwriting qualifs by assignment

22aebd5

Explain why we do not overwrite qualification of locals

6db8c6c

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 Nov 21, 2018

This was referenced Nov 22, 2018

Tracking issue for RFC 2341, Allow locals and destructuring in const fn #48821

Closed

Fix various aspects around let bindings inside const functions #56160

Merged

bors added a commit that referenced this pull request Nov 26, 2018

Auto merge of #56070 - oli-obk:const_let, r=eddyb

b51632e

Allow assignments in const contexts fixes #54098 fixes #51251 fixes #52613

bors merged commit 6db8c6c into rust-lang:master Nov 26, 2018

bors mentioned this pull request Nov 26, 2018

avoid features_untracked #56021

Merged

oli-obk deleted the const_let branch December 17, 2018 14:51

Centril mentioned this pull request Dec 30, 2018

Stabilize let bindings and destructuring in constants and const fn #57175

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow assignments in const contexts #56070

Allow assignments in const contexts #56070

oli-obk commented Nov 19, 2018

rust-highfive commented Nov 19, 2018

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Nov 19, 2018

This comment has been minimized.

RalfJung commented Nov 20, 2018

oli-obk commented Nov 20, 2018

bors commented Nov 20, 2018

eddyb Nov 21, 2018

eddyb commented Nov 21, 2018

oli-obk Nov 21, 2018

eddyb Nov 21, 2018 •

edited

Loading

oli-obk Nov 21, 2018

eddyb Nov 21, 2018

oli-obk Nov 21, 2018

RalfJung commented Nov 21, 2018

rust-highfive commented Nov 21, 2018

oli-obk commented Nov 21, 2018

bors commented Nov 21, 2018

bors commented Nov 26, 2018

bors commented Nov 26, 2018

Allow assignments in const contexts #56070

Allow assignments in const contexts #56070

Conversation

oli-obk commented Nov 19, 2018

rust-highfive commented Nov 19, 2018

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Nov 19, 2018

This comment has been minimized.

RalfJung commented Nov 20, 2018

oli-obk commented Nov 20, 2018

bors commented Nov 20, 2018

eddyb Nov 21, 2018

Choose a reason for hiding this comment

eddyb commented Nov 21, 2018

oli-obk Nov 21, 2018

Choose a reason for hiding this comment

eddyb Nov 21, 2018 • edited Loading

Choose a reason for hiding this comment

oli-obk Nov 21, 2018

Choose a reason for hiding this comment

eddyb Nov 21, 2018

Choose a reason for hiding this comment

oli-obk Nov 21, 2018

Choose a reason for hiding this comment

RalfJung commented Nov 21, 2018

rust-highfive commented Nov 21, 2018

oli-obk commented Nov 21, 2018

bors commented Nov 21, 2018

bors commented Nov 26, 2018

bors commented Nov 26, 2018

eddyb Nov 21, 2018 •

edited

Loading