Skip to content

Translate MIR Repeat (arrays) #29616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 12, 2015
Merged

Translate MIR Repeat (arrays) #29616

merged 3 commits into from
Nov 12, 2015

Conversation

nagisa
Copy link
Member

@nagisa nagisa commented Nov 5, 2015

r? @nikomatsakis

I went ahead and replaced repeat count with a Constant, because it cannot be non-constant to the best of my knowledge.

@nagisa
Copy link
Member Author

nagisa commented Nov 5, 2015

Also, probably fixes #29577?

@dotdash
Copy link
Contributor

dotdash commented Nov 5, 2015

AFAICT let x = [0; 5*5] would mean that the repeat is an operand since the constant expression has not been evaluated yet.

@nagisa
Copy link
Member Author

nagisa commented Nov 5, 2015

Ah, okay. I’ve reverted the Constant part.

@nikomatsakis
Copy link
Contributor

This looks good --- I actually think a constant might have been the right choice here though.

@nikomatsakis
Copy link
Contributor

(We would have to use the const_eval to convert the expression to a constant when building MIR though, not sure what your now-reverted patch was doing.)

@nikomatsakis
Copy link
Contributor

@nagisa do you still have the constant part of your patch somewhere I could see it?

@nagisa
Copy link
Member Author

nagisa commented Nov 9, 2015

@nikomatsakis This is about it (applied over current commit).

diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs
index 23ca221..e351215 100644
--- a/src/librustc_mir/build/expr/as_rvalue.rs
+++ b/src/librustc_mir/build/expr/as_rvalue.rs
@@ -44,8 +44,8 @@ impl<'a,'tcx> Builder<'a,'tcx> {
             }
             ExprKind::Repeat { value, count } => {
                 let value_operand = unpack!(block = this.as_operand(block, value));
-                let count_operand = unpack!(block = this.as_operand(block, count));
-                block.and(Rvalue::Repeat(value_operand, count_operand))
+                let count = this.as_constant(count);
+                block.and(Rvalue::Repeat(value_operand, count))
             }
             ExprKind::Borrow { region, borrow_kind, arg } => {
                 let arg_lvalue = unpack!(block = this.as_lvalue(block, arg));
diff --git a/src/librustc_mir/repr.rs b/src/librustc_mir/repr.rs
index 89b1afa..4ab08f9 100644
--- a/src/librustc_mir/repr.rs
+++ b/src/librustc_mir/repr.rs
@@ -546,7 +546,7 @@ pub enum Rvalue<'tcx> {
     Use(Operand<'tcx>),

     // [x; 32]
-    Repeat(Operand<'tcx>, Operand<'tcx>),
+    Repeat(Operand<'tcx>, Constant<'tcx>),

     // &x or &mut x
     Ref(Region, BorrowKind, Lvalue<'tcx>),
diff --git a/src/librustc_mir/visit.rs b/src/librustc_mir/visit.rs
index b4d6075..9f214af 100644
--- a/src/librustc_mir/visit.rs
+++ b/src/librustc_mir/visit.rs
@@ -134,7 +134,7 @@ pub trait Visitor<'tcx> {

             Rvalue::Repeat(ref value, ref len) => {
                 self.visit_operand(value);
-                self.visit_operand(len);
+                self.visit_constant(len);
             }

             Rvalue::Ref(r, bk, ref path) => {
diff --git a/src/librustc_trans/trans/mir/rvalue.rs b/src/librustc_trans/trans/mir/rvalue.rs
index f3515c0..218edc2 100644
--- a/src/librustc_trans/trans/mir/rvalue.rs
+++ b/src/librustc_trans/trans/mir/rvalue.rs
@@ -51,9 +51,9 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {

             mir::Rvalue::Repeat(ref elem, ref count) => {
                 let elem = self.trans_operand(bcx, elem);
-                let size = self.trans_operand(bcx, count);
+                let size = self.trans_constant(bcx, count);
                 let base = expr::get_dataptr(bcx, lldest);
-                tvec::iter_vec_raw(bcx, base, elem.ty, size.llval, |b, vref, _| {
+                tvec::iter_vec_raw(bcx, base, elem.ty, size, |b, vref, _| {
                     build::Store(b, elem.llval, vref);
                     b
                 })

@nikomatsakis
Copy link
Contributor

@nagisa ok, yeah, I think we should change it to a constant then, since indeed the value must be a compile-time constant

@nagisa
Copy link
Member Author

nagisa commented Nov 10, 2015

@nikomatsakis updated. @dotdash’s case also seems to work without any changes and const-eval in MIR may be expanded as necessary in as_constant.

@nikomatsakis
Copy link
Contributor

@bors r+ thanks!

@bors
Copy link
Collaborator

bors commented Nov 11, 2015

📌 Commit db89a75 has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Nov 11, 2015

⌛ Testing commit db89a75 with merge 94f8304...

@bors
Copy link
Collaborator

bors commented Nov 11, 2015

💔 Test failed - auto-linux-64-x-android-t

@nagisa
Copy link
Member Author

nagisa commented Nov 11, 2015

@nikomatsakis re-review, please.

@nikomatsakis
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Nov 11, 2015

📌 Commit e4e880d has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Nov 12, 2015

⌛ Testing commit e4e880d with merge 35decad...

bors added a commit that referenced this pull request Nov 12, 2015
r? @nikomatsakis

I went ahead and replaced repeat count with a `Constant`, because it cannot be non-constant to the best of my knowledge.
@bors bors merged commit e4e880d into rust-lang:master Nov 12, 2015
@ttemel
Copy link

ttemel commented Nov 13, 2015

Testing commit db89a75 with merge 94f8304...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants