Make raw ptr ops unsafe in const contexts #55009

oli-obk · 2018-10-12T13:09:45Z

rust-highfive · 2018-10-12T15:45:11Z

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.

[00:45:40] .................................................................................................... 2200/4591
[00:45:43] .............i...................................................................................... 2300/4591
[00:45:47] .................................................................................................... 2400/4591
[00:45:50] .................................................................................................... 2500/4591
[00:45:53] ..........................iiiiiiiii................................................................. 2600/4591
[00:45:59] .................................................................................................... 2800/4591
[00:46:02] .................................................................................................... 2900/4591
[00:46:05] ..............................................i..................................................... 3000/4591
[00:46:07] .................................................................................................... 3100/4591
---
travis_time:start:test_mir-opt
Check compiletest suite=mir-opt mode=mir-opt (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:57:19] 
[00:57:19] running 47 tests
[00:57:37] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:501:22
[00:57:37] .............FFF..F....................F..FF...
[00:57:37] 
[00:57:37] ---- [mir-opt] mir-opt/end_region_5.rs stdout ----
[00:57:37] ---- [mir-opt] mir-opt/end_region_5.rs stdout ----
[00:57:37] thread '[mir-opt] mir-opt/end_region_5.rs' panicked at 'Did not find expected line, error: ran out of mir dump to match against
[00:57:37] Expected Line: "fn main::{{closure}}(_1: [closure@NodeId(18) d:&\'14s D]) -> i32 {"
[00:57:37] Test Name: rustc.main-{{closure}}.SimplifyCfg-qualify-consts.after.mir
[00:57:37] ... (elided)
[00:57:37] ... (elided)
[00:57:37] fn main::{{closure}}(_1: [closure@NodeId(18) d:&'14s D]) -> i32 {
[00:57:37]    let mut _0: i32;
[00:57:37]    bb0: {
[00:57:37]        _0 = ((*(_1.0: &'14s D)).0: i32);
[00:57:37]        return;
[00:57:37] Actual:
[00:57:37] Actual:
[00:57:37] closure main::{{closure}}(_1: [closure@NodeId(18) d:&'14s D]) -> i32{
[00:57:37]     let mut _0: i32;
[00:57:37]     bb0: {                              
[00:57:37]         _0 = ((*(_1.0: &'14s D)).0: i32);
[00:57:37]         return;
[00:57:37] }', tools/compiletest/src/runtest.rs:2932:13
[00:57:37] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:57:37] 
[00:57:37] ---- [mir-opt] mir-opt/end_region_6.rs stdout ----
[00:57:37] ---- [mir-opt] mir-opt/end_region_6.rs stdout ----
[00:57:37] thread '[mir-opt] mir-opt/end_region_6.rs' panicked at 'Did not find expected line, error: ran out of mir dump to match against
[00:57:37] Expected Line: "fn main::{{closure}}(_1: [closure@NodeId(22) d:&\'19s D]) -> i32 {"
[00:57:37] Test Name: rustc.main-{{closure}}.SimplifyCfg-qualify-consts.after.mir
[00:57:37] ... (elided)
[00:57:37] ... (elided)
[00:57:37] fn main::{{closure}}(_1: [closure@NodeId(22) d:&'19s D]) -> i32 {
[00:57:37]     let mut _0: i32;
[00:57:37] ... (elided)
[00:57:37]     let _2: &'16_0rs D;
[00:57:37] ... (elided)
[00:57:37]     bb0: {
[00:57:37]         StorageLive(_2);
[00:57:37]         _2 = &'16_0rs (*(_1.0: &'19s D));
[00:57:37]         FakeRead(ForLet, _2);
[00:57:37]         _0 = ((*_2).0: i32);
[00:57:37]         EndRegion('16_0rs);
[00:57:37]         StorageDead(_2);
[00:57:37]         return;
[00:57:37] Actual:
[00:57:37] Actual:
[00:57:37] closure main::{{closure}}(_1: [closure@NodeId(22) d:&'19s D]) -> i32{
[00:57:37]     let mut _0: i32;
[00:57:37]     scope 1 {
[00:57:37]     scope 2 {
[00:57:37]     scope 2 {
[00:57:37]         let _2: &'16_0rs D;
[00:57:37]     }
[00:57:37]     bb0: {                              
[00:57:37]         StorageLive(_2);
[00:57:37]         _2 = &'16_0rs (*(_1.0: &'19s D));
[00:57:37]         FakeRead(ForLet, _2);
[00:57:37]         _0 = ((*_2).0: i32);
[00:57:37]         EndRegion('16_0rs);
[00:57:37]         StorageDead(_2);
[00:57:37]         return;
[00:57:37] }', tools/compiletest/src/runtest.rs:2932:13
[00:57:37] 
[00:57:37] ---- [mir-opt] mir-opt/end_region_7.rs stdout ----
[00:57:37] ---- [mir-opt] mir-opt/end_region_7.rs stdout ----
[00:57:37] thread '[mir-opt] mir-opt/end_region_7.rs' panicked at 'Did not find expected line, error: ran out of mir dump to match against
[00:57:37] Expected Line: "fn main::{{closure}}(_1: [closure@NodeId(22) d:D]) -> i32 {"
[00:57:37] Test Name: rustc.main-{{closure}}.SimplifyCfg-qualify-consts.after.mir
[00:57:37] ... (elided)
[00:57:37] ... (elided)
[00:57:37] fn main::{{closure}}(_1: [closure@NodeId(22) d:D]) -> i32 {
[00:57:37]     let mut _0: i32;
[00:57:37] ... (elided)
[00:57:37]     let _2: &'16_0rs D;
[00:57:37] ... (elided)
[00:57:37]     bb0: {
[00:57:37]         StorageLive(_2);
[00:57:37]         _2 = &'16_0rs (_1.0: D);
[00:57:37]         FakeRead(ForLet, _2);
[00:57:37]         _0 = ((*_2).0: i32);
[00:57:37]         EndRegion('16_0rs);
[00:57:37]         StorageDead(_2);
[00:57:37]         drop(_1) -> [return: bb2, unwind: bb1];
[00:57:37]     bb1: {
[00:57:37]         resume;
[00:57:37]     }
[00:57:37]     bb2: {
[00:57:37]     bb2: {
[00:57:37]         return;
[00:57:37]     }
[00:57:37] }
[00:57:37] Actual:
[00:57:37] closure main::{{closure}}(_1: [closure@NodeId(22) d:D]) -> i32{
[00:57:37]     let mut _0: i32;
[00:57:37]     scope 1 {
[00:57:37]     scope 2 {
[00:57:37]     scope 2 {
[00:57:37]         let _2: &'16_0rs D;
[00:57:37]     }
[00:57:37]     bb0: {                              
[00:57:37]         StorageLive(_2);
[00:57:37]         _2 = &'16_0rs (_1.0: D);
[00:57:37]         FakeRead(ForLet, _2);
[00:57:37]         _0 = ((*_2).0: i32);
[00:57:37]         EndRegion('16_0rs);
[00:57:37]         StorageDead(_2);
[00:57:37]         drop(_1) -> [return: bb2, unwind: bb1];
[00:57:37]     bb1: {
[00:57:37]         resume;
[00:57:37]     }
[00:57:37]     }
[00:57:37]     bb2: {                              
[00:57:37]         return;
[00:57:37] }', tools/compiletest/src/runtest.rs:2932:13
[00:57:37] 
[00:57:37] ---- [mir-opt] mir-opt/end_region_8.rs stdout ----
[00:57:37] ---- [mir-opt] mir-opt/end_region_8.rs stdout ----
[00:57:37] thread '[mir-opt] mir-opt/end_region_8.rs' panicked at 'Did not find expected line, error: ran out of mir dump to match against
[00:57:37] Expected Line: "fn main::{{closure}}(_1: [closure@NodeId(22) r:&\'19s D]) -> i32 {"
[00:57:37] Test Name: rustc.main-{{closure}}.SimplifyCfg-qualify-consts.after.mir
[00:57:37] ... (elided)
[00:57:37] ... (elided)
[00:57:37] fn main::{{closure}}(_1: [closure@NodeId(22) r:&'19s D]) -> i32 {
[00:57:37]     let mut _0: i32;
[00:57:37]     bb0: {
[00:57:37]         _0 = ((*(_1.0: &'21_1rs D)).0: i32);
[00:57:37]         return;
[00:57:37] }
[00:57:37] Actual:
[00:57:37] Actual:
[00:57:37] closure main::{{closure}}(_1: [closure@NodeId(22) r:&'19s D]) -> i32{
[00:57:37]     let mut _0: i32;
[00:57:37]     bb0: {                              
[00:57:37]         _0 = ((*(_1.0: &'21_1rs D)).0: i32);
[00:57:37]         return;
[00:57:37] }', tools/compiletest/src/runtest.rs:2932:13
[00:57:37] 
[00:57:37] ---- [mir-opt] mir-opt/validate_1.rs stdout ----
[00:57:37] ---- [mir-opt] mir-opt/validate_1.rs stdout ----
[00:57:37] thread '[mir-opt] mir-opt/validate_1.rs' panicked at 'Did not find expected line, error: ran out of mir dump to match against
[00:57:37] Expected Line: "fn main::{{closure}}(_1: &ReErased [closure@NodeId(50)], _2: &ReErased mut i32) -> i32 {"
[00:57:37] Test Name: rustc.main-{{closure}}.EraseRegions.after.mir
[00:57:37] ... (elided)
[00:57:37] ... (elided)
[00:57:37] fn main::{{closure}}(_1: &ReErased [closure@NodeId(50)], _2: &ReErased mut i32) -> i32 {
[00:57:37] ... (elided)
[00:57:37]     bb0: {
[00:57:37]         Validate(Acquire, [_1: &ReFree(DefId(0/1:11 ~ validate_1[317d]::main[0]::{{closure}}[0]), BrEnv) [closure@NodeId(50)], _2: &ReFree(DefId(0/1:11 ~ validate_1[317d]::main[0]::{{closure}}[0]), BrAnon(0)) mut i32]);
[00:57:37]         StorageLive(_3);
[00:57:37]         Validate(Suspend(ReScope(Remainder { block: ItemLocalId(25), first_statement_index: 0 })), [(*_2): i32]);
[00:57:37]         _3 = &ReErased (*_2);
[00:57:37]         Validate(Acquire, [(*_3): i32/ReScope(Remainder { block: ItemLocalId(25), first_statement_index: 0 }) (imm)]);
[00:57:37]         _0 = (*_3);
[00:57:37]         EndRegion(ReScope(Remainder { block: ItemLocalId(25), first_statement_index: 0 }));
[00:57:37]         StorageDead(_3);
[00:57:37]         return;
[00:57:37] }
[00:57:37] Actual:
[00:57:37] Actual:
[00:57:37] closure main::{{closure}}(_1: &ReErased [closure@NodeId(50)], _2: &ReErased mut i32) -> i32{
[00:57:37]     let mut _0: i32;
[00:57:37]     scope 1 {
[00:57:37]     scope 2 {
[00:57:37]     scope 2 {
[00:57:37]         let _3: &ReErased i32;
[00:57:37]     }
[00:57:37]     bb0: {                              
[00:57:37]         Validate(Acquire, [_1: &ReFree(DefId(0/1:11 ~ validate_1[317d]::main[0]::{{closure}}[0]), BrEnv) [closure@NodeId(50)], _2: &ReFree(DefId(0/1:11 ~ validate_1[317d]::main[0]::{{closure}}[0]), BrAnon(0)) mut i32]);
[00:57:37]         StorageLive(_3);
[00:57:37]         Validate(Suspend(ReScope(Remainder { block: ItemLocalId(25), first_statement_index: 0})), [(*_2): i32]);
[00:57:37]         _3 = &ReErased (*_2);
[00:57:37]         Validate(Acquire, [(*_3): i32/ReScope(Remainder { block: ItemLocalId(25), first_statement_index: 0}) (imm)]);
[00:57:37]         _0 = (*_3);
[00:57:37]         EndRegion(ReScope(Remainder { block: ItemLocalId(25), first_statement_index: 0}));
[00:57:37]         StorageDead(_3);
[00:57:37]         return;
[00:57:37] }', tools/compiletest/src/runtest.rs:2932:13
[00:57:37] 
[00:57:37] ---- [mir-opt] mir-opt/validate_4.rs stdout ----
[00:57:37] ---- [mir-opt] mir-opt/validate_4.rs stdout ----
[00:57:37] thread '[mir-opt] mir-opt/validate_4.rs' panicked at 'Did not find expected line, error: ran out of mir dump to match against
[00:57:37] Expected Line: "fn write_42::{{closure}}(_1: &ReErased [closure@NodeId(22)], _2: *mut i32) -> () {"
[00:57:37] Test Name: rustc.write_42-{{closure}}.EraseRegions.after.mir
[00:57:37] ... (elided)
[00:57:37] ... (elided)
[00:57:37] fn write_42::{{closure}}(_1: &ReErased [closure@NodeId(22)], _2: *mut i32) -> () {
[00:57:37] ... (elided)
[00:57:37]     bb0: {
[00:57:37]         Validate(Acquire, [_1: &ReFree(DefId(0/1:9 ~ validate_4[317d]::write_42[0]::{{closure}}[0]), BrEnv) [closure@NodeId(22)], _2: *mut i32]);
[00:57:37]         Validate(Release, [_1: &ReFree(DefId(0/1:9 ~ validate_4[317d]::write_42[0]::{{closure}}[0]), BrEnv) [closure@NodeId(22)], _2: *mut i32]);
[00:57:37]         (*_2) = const 23i32;
[00:57:37]         _0 = ();
[00:57:37]         return;
[00:57:37] }
[00:57:37] Actual:
[00:57:37] Actual:
[00:57:37] closure write_42::{{closure}}(_1: &ReErased [closure@NodeId(22)], _2: *mut i32) -> (){
[00:57:37]     let mut _0: ();
[00:57:37]     bb0: {                              
[00:57:37]         Validate(Acquire, [_1: &ReFree(DefId(0/1:9 ~ validate_4[317d]::write_42[0]::{{closure}}[0]), BrEnv) [closure@NodeId(22)], _2: *mut i32]);
[00:57:37]         Validate(Release, [_1: &ReFree(DefId(0/1:9 ~ validate_4[317d]::write_42[0]::{{closure}}[0]), BrEnv) [closure@NodeId(22)], _2: *mut i32]);
[00:57:37]         (*_2) = const 23i32;
[00:57:37]         _0 = ();
[00:57:37]         return;
[00:57:37] }', tools/compiletest/src/runtest.rs:2932:13
[00:57:37] 
[00:57:37] ---- [mir-opt] mir-opt/validate_5.rs stdout ----
[00:57:37] ---- [mir-opt] mir-opt/validate_5.rs stdout ----
[00:57:37] thread '[mir-opt] mir-opt/validate_5.rs' panicked at 'Did not find expected line, error: ran out of mir dump to match against
[00:57:37] Expected Line: "fn main::{{closure}}(_1: &ReErased [closure@NodeId(46)], _2: &ReErased mut i32) -> bool {"
[00:57:37] Test Name: rustc.main-{{closure}}.EraseRegions.after.mir
[00:57:37] ... (elided)
[00:57:37] ... (elided)
[00:57:37] fn main::{{closure}}(_1: &ReErased [closure@NodeId(46)], _2: &ReErased mut i32) -> bool {
[00:57:37] ... (elided)
[00:57:37]     bb0: {
[00:57:37]         Validate(Acquire, [_1: &ReFree(DefId(0/1:9 ~ validate_5[317d]::main[0]::{{closure}}[0]), BrEnv) [closure@NodeId(46)], _2: &ReFree(DefId(0/1:9 ~ validate_5[317d]::main[0]::{{closure}}[0]), BrAnon(0)) mut i32]);
[00:57:37]         StorageLive(_3);
[00:57:37]         StorageLive(_4);
[00:57:37]         StorageLive(_5);
[00:57:37]         Validate(Suspend(ReScope(Node(ItemLocalId(12)))), [(*_2): i32]);
[00:57:37]         _5 = &ReErased mut (*_2);
[00:57:37]         Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(12)))]);
[00:57:37]         _4 = move _5 as *mut i32 (Misc);
[00:57:37]         _3 = move _4;
[00:57:37]         EndRegion(ReScope(Node(ItemLocalId(12))));
[00:57:37]         StorageDead(_4);
[00:57:37]         StorageDead(_5);
[00:57:37]         Validate(Release, [_0: bool, _3: *mut i32]);
[00:57:37]         _0 = const write_42(move _3) -> bb1;
[00:57:37] ... (elided)
[00:57:37] }
[00:57:37] Actual:
[00:57:37] Actual:
[00:57:37] closure main::{{closure}}(_1: &ReErased [closure@NodeId(46)], _2: &ReErased mut i32) -> bool{
[00:57:37]     let mut _0: bool;
[00:57:37]     let mut _3: *mut i32;
[00:57:37]     let mut _4: *mut i32;
[00:57:37]     let mut _5: &ReErased mut i32;
[00:57:37]     bb0: {                              
[00:57:37]         Validate(Acquire, [_1: &ReFree(DefId(0/1:9 ~ validate_5[317d]::main[0]::{{closure}}[0]), BrEnv) [closure@NodeId(46)], _2: &ReFree(DefId(0/1:9 ~ validate_5[317d]::main[0]::{{closure}}[0]), BrAnon(0)) mut i32]);
[00:57:37]         StorageLive(_3);
[00:57:37]         StorageLive(_4);
[00:57:37]         StorageLive(_5);
[00:57:37]         Validate(Suspend(ReScope(Node(ItemLocalId(12)))), [(*_2): i32]);
[00:57:37]         _5 = &ReErased mut (*_2);
[00:57:37]         Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(12)))]);
[00:57:37]         _4 = move _5 as *mut i32 (Misc);inux-gnu" "--mode" "mir-opt" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-5.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "5.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:57:37] 
[00:57:37] 
[00:57:37] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:57:37] Build completed unsuccessfully in 0:16:10
[00:57:37] Build completed unsuccessfully in 0:16:10
[00:57:37] Makefile:58: recipe for target 'check' failed
[00:57:37] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:02bad4c8
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:01753ca9:start=1539359108129548980,finish=1539359108133535706,duration=3986726
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:16d58bf6
$ 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

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)

RalfJung · 2018-10-13T08:28:40Z

Before I go look at the code, a high-level remark: Doesn't this need an RFC or so? Seems like we are slowly deciding on what to do about "unconst", and that's new language design.

RalfJung · 2018-10-13T08:31:35Z

src/librustc_mir/build/mod.rs

        } else {
-            build::construct_const(cx, body_id, return_ty_span)
+            match cx.body_owner_kind {
+                hir::BodyOwnerKind::Fn | hir::BodyOwnerKind::Closure => {


You could reduce the diff and rightwards drift here (and above) my using if let hir::BodyOwnerKind::Fn | hir::BodyOwnerKind::Closure = ....

I don't think that's possible. Or is this a new feature?

Ah, it's an unstable feature. Never mind then.

RalfJung · 2018-10-13T08:34:23Z

src/librustc_mir/transform/check_unsafety.rs

+                    _ => {},
+                }
+            }
+            // raw pointer and fn pointer operations are unsafe


This one (unlike the above) is temporary, right? miri could do all of them. That might be worth noting.

they'll stay unsafe, as you cannot compare the order of pointers to different allocations during const eval.

Okay, the (in)equality operations need to remain unsafe. The only other binop is Offset and that only occurs in generated MIR which, I guess, doesn't go through this check.

Please add a comment why this is unsafe.

src/test/ui/consts/min_const_fn/cmp_fn_pointers.rs

RalfJung · 2018-10-13T08:41:45Z

Just some minors nits in the code, but I am a little concerned about process here. I mean this is all unstable so I guess we have some lee-way, but you did plan to move this through RFC eventually, did you not? @Centril you are more familiar with process, what do you think?

oli-obk · 2018-10-13T08:42:46Z

Before I go look at the code, a high-level remark: Doesn't this need an RFC or so? Seems like we are slowly deciding on what to do about "unconst", and that's new language design.

These are unstable features. We'll need an RFC for the const soundness rules I guess, but once we have that, this is just something that needs an FCP to best of my knowledge

RalfJung · 2018-10-13T08:45:04Z

src/librustc_mir/transform/check_unsafety.rs

+                    }
+                }
+            },
+            // casting pointers to ints is unsafe in const fn


Please add a comment explaining why.

Centril · 2018-10-13T09:24:18Z

Just some minors nits in the code, but I am a little concerned about process here. I mean this is all unstable so I guess we have some lee-way, but you did plan to move this through RFC eventually, did you not? @Centril you are more familiar with process, what do you think?

This seems fine to me since it is unstable and allows for some experimentation on nightly (always useful with some usage experience data). Just as long as we are not making any commitments here.

However, I'd like to check in with the rest of the team before doing this.

@rfcbot poll lang Can we do this experimentally before introducing an RFC?

rfcbot · 2018-10-13T09:24:19Z

RalfJung · 2018-10-13T09:29:39Z

r=me with the nits fixed and when T-lang agrees.

bors · 2018-10-18T01:55:56Z

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

bors · 2018-10-21T14:45:45Z

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

pnkfelix · 2018-10-25T19:17:26Z

(in hindsight i wish we had taken the opportunity with the edition shift to make a whole bunch of casting stuff require unsafe)

nikomatsakis · 2018-10-25T20:57:43Z

My feeling is this:

I am mildly positive on this general direction and willing to approve this particular PR. I do however remain uncertain about the idea of overloading unsafe in this context to mean "may do things we can't statically tell are safe", though I know it makes sense if you screw your head on the right way.

However, I am becoming increasingly nervous about the procedure. Basically the same thing that @RalfJung called out here:

Just some minors nits in the code, but I am a little concerned about process here. I mean this is all unstable so I guess we have some lee-way, but you did plan to move this through RFC eventually, did you not? @Centril you are more familiar with process, what do you think?

For example, in order to figure out what this PR is doing, I have to look at the tests -- there doesn't appear to be any write-up of any kind, even on the PR, much less in something like an RFC. I'm wary of winding up in a situation where our behavior is not well understood apart from the implementation -- imagine if the next Uber comes along to scoop up @oli-obk and @RalfJung and we poor saps were left to puzzle it out on our own! =) Another bad scenario i'd like to avoid is a lot of piecemeal RFCs making small edits — those can make it really hard to understand the "overall vision" if you're not following closely.

To be clear, I'm really critiquing how the lang-team (and Rust team) is functioning more than the consts stuff per se. For example, I feel like there are similar problems around the NLL working group etc, where we ought to be writing up more of the "small decisions" and arriving at a more complete spec. At least consts has the https://github.com/rust-rfcs/const-eval/ repository, which in and of itself is a good start.

I don't entirely know how I think things should be working. I think what I'd like to see is something like the process that I proposed in my blog post. Basically, there would be active discussions on the https://github.com/rust-rfcs/const-eval/ repository on the detailed points. When a rough consensus is reached, it might be surfaced for broader feedback. In this context, I would feel pretty good about "live experimentation" on master, so long as the design is sufficiently feature-gated.

I feel like most of the lang team doesn't really have time to be involved in the nitty gritty, but would maybe like to discuss the "big picture" stuff (to be fair, also, this RFC falls to some extent in that "big picture" category, particularly with respect to whether unsafe is the right keyword here).

oli-obk · 2018-10-25T21:16:06Z

The reason I opened this PR out of nowhere is that I didn't know how to proceed on this topic. Opening a piecemeal RFC for just these two ops felt wrong, too. Additionally these two are the only operations that are unconst but safe.

I am kind of lost on how to proceed. There's no good way of creating a big picture discussion that doesn't immediately dive into discussions about the small pieces. Collecting the small pieces one by one is missing the big picture.

There was some discussion about unconst in rust-lang/const-eval#2 but again, discussions go into details very fast and don't really come back.

So we (meaning @RalfJung ) started building some documents in the const-eval repo. The relevant document for this PR is https://github.com/rust-rfcs/const-eval/blob/master/const_safety.md

We can of course build an image of what we want over there, but what's the endgame? Will we dump all of these as a massive RFC? Start splitting them up again for smaller RFCs? Maybe eRFCs for PRs like this so we can try out stuff?

I wanna do this right, but I don't know how to communicate the problems and features in a way that

everything is covered down to the details
ppl can still read through a gist in a few minutes
everyone gets a chance to have their say

nikomatsakis · 2018-10-25T21:25:27Z

@oli-obk I also don't know the full answer, so don't worry! As I said, I didn't mean my comment as criticism of you in the slightest.

First off, I didn't know about the document pertaining to this, thanks for the link! As I said, the const effort is way ahead of the rest of us. ;)

Regarding the challenge of achieving those three objects, I think the only answer is that we have to have an iterative process.

I think I would feel good about something like this:

We land this PR but there has been a discussion on the "general flavor" of this approach
- For some points of uncertainty, such as whether unsafe or unconst is a better keyword, there is a tracking issue in the repository where discussion can happen

Basically, I think where our current RFC process often goes wrong is that we wind up trying to get full consensus before we start implementing. I would rather see if we can get full consensus on the need to experiment, and also rough consensus on the things we wish to learn from the experiment -- i.e., if we're not sure whether unsafe is the right keyword, let's try it, but with the intention to revisit after the people in the working group have done some validation.

This of course presumes a working group. I suspect we could form up a working group of "people who want to build stuff with const generics" pretty quickly.

oli-obk · 2018-11-02T15:30:04Z

I opened a tracking issue and rebased the PR.

Centril · 2019-01-18T12:38:23Z

From T-Lang's POV, we can go ahead with this now since it would have been FCP-completed by now if I had used @rfcbot merge instead of @rfcbot poll.

oli-obk · 2019-01-21T15:17:11Z

@bors r=RalfJung

bors · 2019-01-21T15:17:13Z

📌 Commit aedc3a5 has been approved by RalfJung

bors · 2019-01-21T18:14:14Z

⌛ Testing commit aedc3a5 with merge ade22eda01eb7678ea6d1abfbb51461973ae2438...

bors · 2019-01-21T21:20:40Z

💔 Test failed - status-appveyor

pietroalbini · 2019-01-21T21:32:46Z

@bors retry

AppVeyor screwed up a bit.

bors · 2019-01-21T23:10:19Z

⌛ Testing commit aedc3a5 with merge 51cc3cd...

@RalfJung

Make raw ptr ops unsafe in const contexts r? @RalfJung cc @Centril

bors · 2019-01-22T02:04:43Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing 51cc3cd to master...

rust-highfive · 2019-01-22T02:05:43Z

📣 Toolstate changed by #55009!

Tested on commit 51cc3cd.
Direct link to PR: #55009

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).

@Manishearth

Tested on commit rust-lang/rust@51cc3cd. Direct link to PR: <rust-lang/rust#55009> 💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). 💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).

flip1995 · 2019-01-22T14:58:17Z

src/librustc/hir/map/mod.rs

+            Node::Expr(&Expr { node: ExprKind::Closure(..), .. }) => {
+                BodyOwnerKind::Closure
+            }
+            node => bug!("{:#?} is not a body node", node),


This currently breaks Clippy in clippy_lints/src/utils/mod.rs:

pub fn in_constant(cx: &LateContext<'_, '_>, id: NodeId) -> bool { let parent_id = cx.tcx.hir().get_parent(id); match cx.tcx.hir().body_owner_kind(parent_id) { hir::BodyOwnerKind::Fn | hir::BodyOwnerKind::Closure => false, hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(..) => true, } }

This is because it is called on an enum variant here: clippy_lints/src/non_copy_const.rs. self.get(id) returns Node::Item(Item { node: ItemKind::Enum(..), .. })

Should this function really produce an ICE if the node isn't one of the above. I think the previous approach of just assuming it is a function is also wrong. Maybe another variant for BodyOwnerKind would be better (BodyOwnerKind::Other?)

I would be happy to open a PR if this refactor should be done. Otherwise I'll try to fix this in Clippy.

cc @oli-obk

We could make it return an Option, but I'm not sure if that is desirable. We didn't want to know whether something was a function, closure or something else, we just wanted to know whether it is a constant or static.

In the end I decided to just check for const/static manually: https://github.com/rust-lang/rust-clippy/pull/3685/files

rust-highfive assigned RalfJung Oct 12, 2018

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

This comment has been minimized.

Sign in to view

RalfJung reviewed Oct 13, 2018

View reviewed changes

src/test/ui/consts/min_const_fn/cmp_fn_pointers.rs Outdated Show resolved Hide resolved

RalfJung reviewed Oct 13, 2018

View reviewed changes

Centril added the I-nominated label Oct 13, 2018

oli-obk force-pushed the const_safety branch from b6d4672 to 5998a34 Compare October 18, 2018 10:40

oli-obk added T-lang Relevant to the language team, which will review and decide on the PR/issue. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Oct 25, 2018

Centril removed the I-nominated label Oct 25, 2018

oli-obk mentioned this pull request Oct 25, 2018

What do we do with "unconst" operations? rust-lang/const-eval#14

Open

oli-obk force-pushed the const_safety branch from 5998a34 to 70f6e85 Compare November 2, 2018 11:50

This comment has been minimized.

Sign in to view

oli-obk force-pushed the const_safety branch from 70f6e85 to d8a50c9 Compare November 2, 2018 15:55

Centril added I-nominated and removed I-nominated labels Jan 3, 2019

Differentiate between closure and function bodies

64afc6b

oli-obk force-pushed the const_safety branch 2 times, most recently from bfee610 to eee155c Compare January 21, 2019 10:53

Declare some unconst operations as unsafe in const fn

aedc3a5

oli-obk force-pushed the const_safety branch from eee155c to aedc3a5 Compare January 21, 2019 15:02

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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jan 21, 2019

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

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 Jan 21, 2019

bors added a commit that referenced this pull request Jan 21, 2019

Auto merge of #55009 - oli-obk:const_safety, r=RalfJung

51cc3cd

Make raw ptr ops unsafe in const contexts r? @RalfJung cc @Centril

bors merged commit aedc3a5 into rust-lang:master Jan 22, 2019

flip1995 reviewed Jan 22, 2019

View reviewed changes

oli-obk mentioned this pull request Jan 28, 2019

Storing a function pointer in a const usize works on nightly #51559

Closed

oli-obk deleted the const_safety branch June 15, 2020 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make raw ptr ops unsafe in const contexts #55009

Make raw ptr ops unsafe in const contexts #55009

oli-obk commented Oct 12, 2018

This comment has been minimized.

rust-highfive commented Oct 12, 2018

RalfJung commented Oct 13, 2018

RalfJung Oct 13, 2018

oli-obk Oct 13, 2018

RalfJung Oct 13, 2018 •

edited

Loading

RalfJung Oct 13, 2018

oli-obk Oct 13, 2018

RalfJung Oct 13, 2018

RalfJung commented Oct 13, 2018

oli-obk commented Oct 13, 2018

RalfJung Oct 13, 2018

Centril commented Oct 13, 2018

rfcbot commented Oct 13, 2018 •

edited by aturon

Loading

RalfJung commented Oct 13, 2018

bors commented Oct 18, 2018

bors commented Oct 21, 2018

pnkfelix commented Oct 25, 2018 •

edited

Loading

nikomatsakis commented Oct 25, 2018

oli-obk commented Oct 25, 2018

nikomatsakis commented Oct 25, 2018

oli-obk commented Nov 2, 2018

This comment has been minimized.

Centril commented Jan 18, 2019

oli-obk commented Jan 21, 2019

bors commented Jan 21, 2019

bors commented Jan 21, 2019

bors commented Jan 21, 2019

pietroalbini commented Jan 21, 2019

bors commented Jan 21, 2019

bors commented Jan 22, 2019

rust-highfive commented Jan 22, 2019

flip1995 Jan 22, 2019

oli-obk Jan 22, 2019

Make raw ptr ops unsafe in const contexts #55009

Make raw ptr ops unsafe in const contexts #55009

Conversation

oli-obk commented Oct 12, 2018

This comment has been minimized.

rust-highfive commented Oct 12, 2018

RalfJung commented Oct 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung Oct 13, 2018 • edited Loading

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

RalfJung commented Oct 13, 2018

oli-obk commented Oct 13, 2018

Choose a reason for hiding this comment

Centril commented Oct 13, 2018

rfcbot commented Oct 13, 2018 • edited by aturon Loading

RalfJung commented Oct 13, 2018

bors commented Oct 18, 2018

bors commented Oct 21, 2018

pnkfelix commented Oct 25, 2018 • edited Loading

nikomatsakis commented Oct 25, 2018

oli-obk commented Oct 25, 2018

nikomatsakis commented Oct 25, 2018

oli-obk commented Nov 2, 2018

This comment has been minimized.

Centril commented Jan 18, 2019

oli-obk commented Jan 21, 2019

bors commented Jan 21, 2019

bors commented Jan 21, 2019

bors commented Jan 21, 2019

pietroalbini commented Jan 21, 2019

bors commented Jan 21, 2019

bors commented Jan 22, 2019

rust-highfive commented Jan 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung Oct 13, 2018 •

edited

Loading

rfcbot commented Oct 13, 2018 •

edited by aturon

Loading

pnkfelix commented Oct 25, 2018 •

edited

Loading