implementing RFC 1623. This fixes #35897. #35915
Conversation
This is a work in progress. In particular, I want to add more tests, especially the compile-fail test is very bare-bones.
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
|
Now the tests pass for me. That was too easy. 😄 I still want to add more tests to check for possible interference with custom types (structs/enums) and aliases, but @nikomatsakis if you want to merge this, I'm fine with adding those tests in a later PR. |
| @@ -1554,7 +1554,7 @@ fn type_of_def_id<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, | |||
| NodeItem(item) => { | |||
| match item.node { | |||
| ItemStatic(ref t, _, _) | ItemConst(ref t, _) => { | |||
There was a problem hiding this comment.
I forget, did the RFC cover both constants and statics? Presumably yes.
Looks good to me thus far -- why not add the tests now though. |
...there is still one confusing thing – see the _BAZ functions, which appear to be elided in the `compile-fail` test and defaulted in the ´run-pass` test (if you uncomment line 73).
|
Note there's still a strange mismatch between the run-pass and the compile-fail test. |
llogiq
changed the title
|
|
||
| let y = &[1u8, 2, 3]; | ||
| STATIC_BAZ(BYTES); | ||
| //CONST_BAZ(y); // strangely enough, this fails |
There was a problem hiding this comment.
can you elaborate? how does it fail?
There was a problem hiding this comment.
ah, I created a play version: https://is.gd/RBg85e and I see now what you mean. This is, I think, expected! The type of Baz would be &'static fn(&'static [u8]) -> Option<u8>, and yet when you supply y it only has a lifetime confined to the current stack frame. If you change to CONST_BAZ(BYTES) it should work, I think.
|
cc @chriskrycho This will also need some documentation. I vaguely remember you wanted to be notified in such an event. I'm not sure, but perhaps a paragraph in the relevant book chapter? |
|
Error appears to be LLVM related. |
|
Thanks, @TimNN |
|
👍 @llogiq – I'll review the RFC and see if I can submit a PR to the book in the next two weeks. (That's a long time because lots of travel and other commitments in the meantime, but it's on my radar now.) |
|
@chriskrycho Great, one thing I don't need to do. To save you time, the RFC basically boils down to "elide lifetimes with a |
|
@nikomatsakis now that #36080 has landed, can we bump travis? Should I make a "bump" commit? |
|
@llogiq I don't know much about how travis works in that respect. |
|
Er, do you just want to re-run travis results? |
|
@bors r+ The travis failures look to be unrelated to this patch. |
|
📌 Commit a87b4d8 has been approved by |
implementing RFC 1623. This fixes #35897. This is a work in progress. In particular, I want to add more tests, especially the compile-fail test is very bare-bones.
|
@nikomatsakis |
| @@ -0,0 +1,98 @@ | |||
| // Copyright 2012 The Rust Project Developers. See the COPYRIGHT | |||
There was a problem hiding this comment.
Umm this seems to be a rustfmt-generated "backup" file, surely that shouldn't have been committed?
|
@petrochenkov argh you are correct -- that was utterly wrong on my part. We should totally feature gate this. @llogiq, can I walk you through landing that change? |
|
And, yes, the /me embarassed |
|
(Adding the feature-gate itself is probably harder, of course, than the rest of this change, but not overly hard. The easiest way I think is to configure the rscope (or make a new one) that checks the feature gate and, when return |
|
/me=embarrassed, too. |
|
Nah, no big thing. :) Sharp eyes though, @petrochenkov. |
This is a work in progress. In particular, I want to add more tests,
especially the compile-fail test is very bare-bones.