Allow mutable slices in statics. #11797
Conversation
| }, | ||
| ExprVstore(_, ExprVstoreUniq) | | ||
| ExprVstore(_, ExprVstoreBox) => { | ||
| sess.span_err(e.span, "cannot allocate vectors in constant expressions") | ||
| sess.span_err(e.span, "cannot allocate vectors in constant expressions") |
There was a problem hiding this comment.
Rust uses 4-spaces indent afaik.
There was a problem hiding this comment.
Made it consistent with rest of file.
There was a problem hiding this comment.
Let's leave this as 4 spaces and hope the rest of the file gets updated soon.
|
cc @nikomatsakis this has some interactions with borrowck that need to be resolved, possibly with special cases for static muts. Or, we could just not allow mutable slices, which would make me sad (and probably @Tobba too) |
|
Test currently fails with: This appears to be due to all statics being considered AliasableOther (librustc/middle/mem_categorization.rs:1176). |
| llvm::LLVMAddGlobal(cx.llmod, llty.to_ref(), name) | ||
| }); | ||
| llvm::LLVMSetInitializer(gv, cv); | ||
| llvm::LLVMSetGlobalConstant(gv, True); |
There was a problem hiding this comment.
This seems fishy, it's a mutable static but you're setting it as constant!
You may want to test fancier cases as well, you want to make sure that nothing is recursively constant as well. Something like:
struct Foo { a: int };
static mut A: &'static mut [Foo] = &'static mut [Foo { a : 3 }];
fn main() {
A[0].a += 3;
}
|
Is there an issue #? What exactly is being permitted here? |
|
Yes - #11411 |
|
Builds and tests cleanly now. Appears to actually work... |
| fn visit_pat(&mut self, p: &Pat, env: bool) { | ||
| check_pat(self, p, env); | ||
| } | ||
| fn visit_item(&mut self, item: &Item, env: bool) { | ||
| check_item(self, self.sess, self.ast_map, self.def_map, self.method_map, |
There was a problem hiding this comment.
Why do all these need to be passed separately? Can't check_item just retrieve them from self itself?
There was a problem hiding this comment.
That's how it was being done previously. Should I change it?
There was a problem hiding this comment.
Oh, I see now... I got distracted by the extra diff. I guess there's no strong need to change it, although it does look kinda silly.
(Maybe you could move this back to being before visit_pat?)
|
Please do not merge until I have time to review, thanks. |
|
ping @nikomatsakis |
|
Oh yes, sorry... |
|
ping @nikomatsakis :( |
|
OK, I do apologize for taking so long. This will need a rebase. I read it over. It seems fine, though I still am not happy about putting an |
|
@nikomatsakis I've rebased, but I had to make more changes to borrowck to get it to work. Do these look okay? |
|
|
||
| static mut TEST: &'static mut [int] = &mut [1]; | ||
|
|
||
| fn main() { |
There was a problem hiding this comment.
Just a drive-by-comment, but to pass the check-fast target on windows/bsd this will need to be pub fn main
|
@xales are you planning you work on this? If not, do you mind if I take over? |
|
superseded by #12742 |
No description provided.