Stream: t-compiler/wg-nll

Topic: #51269 ReStatic ICE with nll and thread_local


nikomatsakis (Sep 18 2018 at 21:27, on Zulip):

so we decided to defer this... what is needed is someone to add a regression test I guess. Pretty easy, I suppose.

I am wondering if it is worth trying to make a more "comprehensive" set of regression tests around static and static mut. We definitely already have some, I suppose. But obviously at least one check-box on the matrix was not included.

nikomatsakis (Sep 18 2018 at 21:28, on Zulip):

cc @Santiago Pastorino simple task but it'd be good to get it done

Santiago Pastorino (Sep 18 2018 at 21:34, on Zulip):

:+1:

Santiago Pastorino (Sep 19 2018 at 19:15, on Zulip):

@nikomatsakis about this task

Santiago Pastorino (Sep 19 2018 at 19:16, on Zulip):

I see in the issue there are a bunch of possible tests

Santiago Pastorino (Sep 19 2018 at 19:16, on Zulip):

is there one that sounds better to you?

nikomatsakis (Sep 19 2018 at 19:27, on Zulip):

well, we could start with the original test in the issue

#![feature(nll)]
#![feature(thread_local)]

#[thread_local] static mut X1:u64 = 0;

struct S1 {
    a:&'static mut u64,
}

impl S1 {
    fn new(_x:u64) -> S1 {
        S1 { a:unsafe { &mut X1 } }
    }
}

fn main() {
    S1::new(0).a;
}
nikomatsakis (Sep 19 2018 at 19:27, on Zulip):

that's the minimum we should add I suppose

nikomatsakis (Sep 19 2018 at 19:28, on Zulip):

@Santiago Pastorino it should be a "sibling" to this existing test, I think:

src/test/ui/borrowck/borrowck-thread-local-static-borrow-outlives-fn.rs
nikomatsakis (Sep 19 2018 at 19:28, on Zulip):

that one tests non-mut

Santiago Pastorino (Sep 19 2018 at 20:41, on Zulip):

ok

Santiago Pastorino (Sep 19 2018 at 20:41, on Zulip):

@nikomatsakis so this should compile on nll?

Santiago Pastorino (Sep 19 2018 at 20:42, on Zulip):

and in that case, don't remember when something compiles what to do on ui tests

Santiago Pastorino (Sep 19 2018 at 20:42, on Zulip):

or should I put it on run-pass?

nikomatsakis (Sep 19 2018 at 20:42, on Zulip):

you should put it in ui/nll or whatever I think

nikomatsakis (Sep 19 2018 at 20:42, on Zulip):

with a comment like // run-pass

nikomatsakis (Sep 19 2018 at 20:42, on Zulip):

however, we should also add some explanatory comment

nikomatsakis (Sep 19 2018 at 20:43, on Zulip):

in particular, it passes now but it's not clear that it should

nikomatsakis (Sep 19 2018 at 20:43, on Zulip):

I should file an issue and you can put FIXME(#12345)

nikomatsakis (Sep 19 2018 at 20:43, on Zulip):

let me write that issue :)

Santiago Pastorino (Sep 19 2018 at 20:43, on Zulip):

ahh right

Santiago Pastorino (Sep 19 2018 at 20:45, on Zulip):

done

Santiago Pastorino (Sep 19 2018 at 20:45, on Zulip):

well need to write a comment

nikomatsakis (Sep 19 2018 at 20:50, on Zulip):

you can tag it as FIXME(#54366) @Santiago Pastorino

nikomatsakis (Sep 19 2018 at 20:50, on Zulip):

(just filed)

Santiago Pastorino (Sep 19 2018 at 20:52, on Zulip):

:+1:

Santiago Pastorino (Sep 19 2018 at 20:52, on Zulip):

@nikomatsakis should I reuse some text from the issue?

Santiago Pastorino (Sep 19 2018 at 20:52, on Zulip):

for the comment, I meant

nikomatsakis (Sep 19 2018 at 20:52, on Zulip):

if you like. Probably don't have to copy too much. Maybe something like:

FIXME(#54366) — We probably shouldn't allow #[thread_local] static mut to get a 'static lifetime.

Santiago Pastorino (Sep 19 2018 at 20:56, on Zulip):

https://github.com/rust-lang/rust/pull/54367

Santiago Pastorino (Sep 19 2018 at 20:57, on Zulip):

ouch, I've added a Fixes statement

Santiago Pastorino (Sep 19 2018 at 20:57, on Zulip):

you want me to add FIXME(#54366), right?

Santiago Pastorino (Sep 19 2018 at 20:57, on Zulip):

so it just references that issue

Santiago Pastorino (Sep 19 2018 at 20:57, on Zulip):

thought you were going to create an add regression test issue

Santiago Pastorino (Sep 19 2018 at 20:58, on Zulip):

updated to FIXME :)

nikomatsakis (Sep 19 2018 at 21:04, on Zulip):

thought you were going to create an add regression test issue

yeah well I suspect we have a lot of the tests I would want already but...

nikomatsakis (Sep 19 2018 at 21:05, on Zulip):

at least we have the one around the lifetime of #[thread_local] static

Santiago Pastorino (Sep 19 2018 at 21:13, on Zulip):

:+1:

Last update: Nov 21 2019 at 13:10UTC