Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Static array and array literal improvements #375

Merged
merged 5 commits into from

5 participants

@9rnsr
Collaborator
  • Issue 3703 - static array assignment
    Check static-array length matching on assign expression.

  • Issue 5290 - Static array literals with too few elements
    Check static-array length matching on initialiging.

  • Issue 6470 - postblits not called on arrays of structs
    Call postblits of array literal elements correctly.

  • Issue 2356 - array literal as non static initializer generates horribly inefficient code
    If array literal is used as static-array initializer, the initializing is converted as initializings of elements.
    An initializing of static-array
    S[n] sa = [e0, e1, ..., en-1];
    is converted to
    sa[0] = e0, sa[1] = e1, ..., sa[n-1] = en-1;

  • Issue 6636 - Destructors of static array elements are not called on function parameter

  • Issue 6637 - Postblits of static array elements are not called on function argument

@donc
Collaborator

Your fix for bug 2356 is good when the array is small (length <= about 8), but a full fix would check if all elements are compile-time constants. If they are, create a const initializer, and blit it across.
There are some other optimization opportunities (eg, if most members of the array literals are zeros, or if there are only a couple of non-constants, perform a blit or zero-init first, and then fill in the gaps).

Also note that CTFE can create an array literal as well, so the transformation needs to happen in the glue layer, not in expression.c.

@9rnsr
Collaborator

Also note that CTFE can create an array literal as well, so the transformation needs to happen in the glue layer, not in expression.c.

I was able to move static array construction codes into AssignExp::toElem(), but I wasn't able to work the interpret() correctly.
Currently, it is difficult to me changing codes of interpret.c.

@donc
Collaborator

Interpret.c should not be changed. Array literals MUST survive until the glue layer.

@9rnsr
Collaborator

glue layer == e2ir.c ? (I think expression.c is front-end, and src/backend is backend layer.)
If so, I was able to survive array literals until the glue layer, see commit 662b5a3. But I that also means that survives array literals until interpret layer. At least, I had to change interpret.c to compile small sample code.

@9rnsr
Collaborator

Remove fixing 5290. Because it is not bug fix, it is enhancement.

@Trass3r

What prevents this from being merged?
Have all of Don's suggestions been implemented?

@9rnsr
Collaborator

Have all of Don's suggestions been implemented?

No, and I have no schedule to implement it in near future.

@yebblies
Collaborator

Could you please split this into several pull requests so the smaller issues can be fixed?

9rnsr added some commits
@9rnsr 9rnsr Scanning past comma on AssignExp::e2 is not need, because it is alrea…
…dy processed at the starting of semantic.
aa7e809
@9rnsr 9rnsr Issue 3703 - static array assignment 6dfdd2d
@9rnsr 9rnsr Issue 6636 - Destructors of static array elements are not called on f…
…unction parameter

Call dtor of static array parameter at function scope end.
57d7f41
@9rnsr 9rnsr Issue 6637 - Postblits of static array elements are not called on fun…
…ction argument

call element postblits of static array on function argument
2f09427
@9rnsr 9rnsr Issue 6470 - postblits not called on arrays of structs
- Call postblits on array slice assign
- Call postblits on array literal elements
26aacf0
@9rnsr
Collaborator

Remove commit for bug 2356, and reconstructed commits.
Now, there are fixes for only postblit/destructor and array literal/static array problems.

@WalterBright WalterBright merged commit 385fd46 into D-Programming-Language:master
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@AlexeyProkhin AlexeyProkhin referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 5, 2012
  1. @9rnsr

    Scanning past comma on AssignExp::e2 is not need, because it is alrea…

    9rnsr authored
    …dy processed at the starting of semantic.
  2. @9rnsr
  3. @9rnsr

    Issue 6636 - Destructors of static array elements are not called on f…

    9rnsr authored
    …unction parameter
    
    Call dtor of static array parameter at function scope end.
  4. @9rnsr

    Issue 6637 - Postblits of static array elements are not called on fun…

    9rnsr authored
    …ction argument
    
    call element postblits of static array on function argument
  5. @9rnsr

    Issue 6470 - postblits not called on arrays of structs

    9rnsr authored
    - Call postblits on array slice assign
    - Call postblits on array literal elements
This page is out of date. Refresh to see the latest.
View
2  src/e2ir.c
@@ -2753,7 +2753,7 @@ elem *AssignExp::toElem(IRState *irs)
/* Determine if we need to do postblit
*/
int postblit = 0;
- if (needsPostblit(t1))
+ if (needsPostblit(t1->nextOf()))
postblit = 1;
assert(e2->type->ty != Tpointer);
View
103 src/expression.c
@@ -770,24 +770,41 @@ void valueNoDtor(Expression *e)
#if DMDV2
Expression *callCpCtor(Loc loc, Scope *sc, Expression *e, int noscope)
{
+ if (e->op == TOKarrayliteral)
+ {
+ ArrayLiteralExp *ae = (ArrayLiteralExp *)e;
+ for (size_t i = 0; i < ae->elements->dim; i++)
+ {
+ ae->elements->tdata()[i] =
+ callCpCtor(loc, sc, ae->elements->tdata()[i], noscope);
+ }
+ e = ae->semantic(sc);
+ return e;
+ }
+
Type *tb = e->type->toBasetype();
- assert(tb->ty == Tstruct);
- StructDeclaration *sd = ((TypeStruct *)tb)->sym;
- if (sd->cpctor)
- {
- /* Create a variable tmp, and replace the argument e with:
- * (tmp = e),tmp
- * and let AssignExp() handle the construction.
- * This is not the most efficent, ideally tmp would be constructed
- * directly onto the stack.
- */
- Identifier *idtmp = Lexer::uniqueId("__cpcttmp");
- VarDeclaration *tmp = new VarDeclaration(loc, tb, idtmp, new ExpInitializer(0, e));
- tmp->storage_class |= STCctfe;
- tmp->noscope = noscope;
- Expression *ae = new DeclarationExp(loc, tmp);
- e = new CommaExp(loc, ae, new VarExp(loc, tmp));
- e = e->semantic(sc);
+ Type *tv = tb;
+ while (tv->ty == Tsarray)
+ tv = tv->nextOf()->toBasetype();
+ if (tv->ty == Tstruct)
+ {
+ StructDeclaration *sd = ((TypeStruct *)tv)->sym;
+ if (sd->cpctor)
+ {
+ /* Create a variable tmp, and replace the argument e with:
+ * (tmp = e),tmp
+ * and let AssignExp() handle the construction.
+ * This is not the most efficent, ideally tmp would be constructed
+ * directly onto the stack.
+ */
+ Identifier *idtmp = Lexer::uniqueId("__cpcttmp");
+ VarDeclaration *tmp = new VarDeclaration(loc, tb, idtmp, new ExpInitializer(0, e));
+ tmp->storage_class |= STCctfe;
+ tmp->noscope = noscope;
+ Expression *ae = new DeclarationExp(loc, tmp);
+ e = new CommaExp(loc, ae, new VarExp(loc, tmp));
+ e = e->semantic(sc);
+ }
}
return e;
}
@@ -1077,15 +1094,22 @@ Type *functionParameters(Loc loc, Scope *sc, TypeFunction *tf,
}
Type *tb = arg->type->toBasetype();
-#if !SARRAYVALUE
- // Convert static arrays to pointers
- if (tb->ty == Tsarray)
+ if (arg->op == TOKarrayliteral)
{
- arg = arg->checkToPointer();
+ arg = callCpCtor(loc, sc, arg, 1);
}
+ else if (tb->ty == Tsarray)
+ {
+#if !SARRAYVALUE
+ // Convert static arrays to pointers
+ arg = arg->checkToPointer();
+#else
+ // call copy constructor of each element
+ arg = callCpCtor(loc, sc, arg, 1);
#endif
+ }
#if DMDV2
- if (tb->ty == Tstruct && !(p->storageClass & (STCref | STCout)))
+ else if (tb->ty == Tstruct && !(p->storageClass & (STCref | STCout)))
{
if (arg->op == TOKcall)
{
@@ -10361,16 +10385,6 @@ Expression *AssignExp::semantic(Scope *sc)
sd->cpctor)
{ /* We have a copy constructor for this
*/
- // Scan past commma's
- Expression *ec = NULL;
- while (e2->op == TOKcomma)
- { CommaExp *ecomma = (CommaExp *)e2;
- e2 = ecomma->e2;
- if (ec)
- ec = new CommaExp(ecomma->loc, ec, ecomma->e1);
- else
- ec = ecomma->e1;
- }
if (e2->op == TOKquestion)
{ /* Write as:
* a ? e1 = b : e1 = c;
@@ -10381,8 +10395,6 @@ Expression *AssignExp::semantic(Scope *sc)
AssignExp *ea2 = new AssignExp(econd->e1->loc, e1, econd->e2);
ea2->op = op;
Expression *e = new CondExp(loc, econd->econd, ea1, ea2);
- if (ec)
- e = new CommaExp(loc, ec, e);
return e->semantic(sc);
}
else if (e2->op == TOKvar ||
@@ -10398,8 +10410,6 @@ Expression *AssignExp::semantic(Scope *sc)
Expression *e = new DotVarExp(loc, e1, sd->cpctor, 0);
e = new CallExp(loc, e, e2);
- if (ec)
- e = new CommaExp(loc, ec, e);
return e->semantic(sc);
}
else if (e2->op == TOKcall)
@@ -10424,6 +10434,21 @@ Expression *AssignExp::semantic(Scope *sc)
if (t1->ty == Tsarray && !refinit)
{
+ Type *t2 = e2->type->toBasetype();
+
+ if (t2->ty == Tsarray && !t2->implicitConvTo(t1->nextOf()))
+ { // static array assignment should check their lengths
+ TypeSArray *tsa1 = (TypeSArray *)t1;
+ TypeSArray *tsa2 = (TypeSArray *)t2;
+ uinteger_t dim1 = tsa1->dim->toInteger();
+ uinteger_t dim2 = tsa2->dim->toInteger();
+ if (dim1 != dim2)
+ {
+ error("mismatched array lengths, %d and %d", (int)dim1, (int)dim2);
+ return new ErrorExp();
+ }
+ }
+
if (e1->op == TOKindex &&
((IndexExp *)e1)->e1->type->toBasetype()->ty == Taarray)
{
@@ -10435,7 +10460,6 @@ Expression *AssignExp::semantic(Scope *sc)
}
else
{
- Type *t2 = e2->type->toBasetype();
// Convert e2 to e2[], unless e2-> e1[0]
if (t2->ty == Tsarray && !t2->implicitConvTo(t1->nextOf()))
{
@@ -10454,6 +10478,11 @@ Expression *AssignExp::semantic(Scope *sc)
if (!e2->rvalue())
return new ErrorExp();
+ if (e2->op == TOKarrayliteral)
+ {
+ e2 = callCpCtor(loc, sc, e2, 1);
+ }
+
if (e1->op == TOKarraylength)
{
// e1 is not an lvalue, but we let code generator handle it
View
2  src/func.c
@@ -1577,12 +1577,14 @@ void FuncDeclaration::semantic3(Scope *sc)
if (v->storage_class & (STCref | STCout))
continue;
+#if !SARRAYVALUE
/* Don't do this for static arrays, since static
* arrays are called by reference. Remove this
* when we change them to call by value.
*/
if (v->type->toBasetype()->ty == Tsarray)
continue;
+#endif
if (v->noscope)
continue;
View
9 test/fail_compilation/fail3703.d
@@ -0,0 +1,9 @@
+// Issue 3703 - static array assignment
+
+void main()
+{
+ int[1] a = [1];
+ int[2] b;
+
+ b = a; // should make compile error
+}
View
75 test/runnable/sdtor.d
@@ -1878,6 +1878,78 @@ void test6177()
/**********************************/
+// 6470
+
+struct S6470
+{
+ static int spblit;
+
+ this(this){ ++spblit; }
+}
+
+void test6470()
+{
+ S6470[] a1;
+ S6470[] a2;
+ a1.length = 3;
+ a2.length = 3;
+ a1[] = a2[];
+ assert(S6470.spblit == 3);
+
+ S6470 s;
+
+ S6470[] a3;
+ a3.length = 3;
+ a3 = [s, s, s];
+ assert(S6470.spblit == 6);
+
+ void func(S6470[] a){}
+ func([s, s, s]);
+ assert(S6470.spblit == 9);
+}
+
+/**********************************/
+// 6636
+
+struct S6636
+{
+ ~this()
+ {
+ ++sdtor;
+ }
+}
+
+void func6636(S6636[3] sa) {}
+
+void test6636()
+{
+ sdtor = 0;
+
+ S6636[3] sa;
+ func6636(sa);
+ assert(sdtor == 3);
+}
+
+/**********************************/
+// 6637
+
+struct S6637
+{
+ static int spblit;
+
+ this(this){ ++spblit; }
+}
+
+void test6637()
+{
+ void func(S6637[3] sa){}
+
+ S6637[3] sa;
+ func(sa);
+ assert(S6637.spblit == 3);
+}
+
+/**********************************/
// 7353
struct S7353
@@ -1984,6 +2056,9 @@ int main()
test60();
test4316();
test6177();
+ test6470();
+ test6636();
+ test6637();
test7353();
printf("Success\n");
Something went wrong with that request. Please try again.