Skip to content

Commit

Permalink
Merge pull request #1465 from 9rnsr/fix3825
Browse files Browse the repository at this point in the history 
Issue 3825 - AAs entries are default initialized before the new value is evaluated
  • Loading branch information
@WalterBright
WalterBright committed Mar 4, 2013
2 parents d2e085f + d5aac8c commit 4af7946
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 9 deletions.
78 changes: 69 additions & 9 deletions src/expression.c
View file
Expand Up @@ -6364,8 +6364,7 @@ Expression *BinAssignExp::semantic(Scope *sc)
e = e->semantic(sc);
return e;
}

if (e1->op == TOKslice)
else if (e1->op == TOKslice)
{
// T[] op= ...
e = typeCombine(sc);
Expand Down Expand Up @@ -6431,7 +6430,8 @@ Expression *BinAssignExp::semantic(Scope *sc)
if (e1->op == TOKerror || e2->op == TOKerror)
return new ErrorExp();

return checkComplexOpAssign(sc);
checkComplexOpAssign(sc);
return reorderSettingAAElem(sc);
}

#if DMDV2
Expand Down Expand Up @@ -10828,7 +10828,7 @@ Expression *AssignExp::semantic(Scope *sc)

type = e1->type;
assert(type);
return this;
return reorderSettingAAElem(sc);
}

Expression *AssignExp::checkToBoolean(Scope *sc)
Expand Down Expand Up @@ -10913,7 +10913,6 @@ Expression *CatAssignExp::semantic(Scope *sc)
checkPostblit(e1->loc, tb1next);
e2 = e2->castTo(sc, e1->type);
type = e1->type;
e = this;
}
else if ((tb1->ty == Tarray) &&
e2->implicitConvTo(tb1next)
Expand All @@ -10922,7 +10921,6 @@ Expression *CatAssignExp::semantic(Scope *sc)
checkPostblit(e2->loc, tb2);
e2 = e2->castTo(sc, tb1next);
type = e1->type;
e = this;
}
else if (tb1->ty == Tarray &&
(tb1next->ty == Tchar || tb1next->ty == Twchar) &&
Expand All @@ -10932,7 +10930,6 @@ Expression *CatAssignExp::semantic(Scope *sc)
{ // Append dchar to char[] or wchar[]
e2 = e2->castTo(sc, Type::tdchar);
type = e1->type;
e = this;

/* Do not allow appending wchar to char[] because if wchar happens
* to be a surrogate pair, nothing good can result.
Expand All @@ -10942,9 +10939,9 @@ Expression *CatAssignExp::semantic(Scope *sc)
{
if (tb1 != Type::terror && tb2 != Type::terror)
error("cannot append type %s to type %s", tb2->toChars(), tb1->toChars());
e = new ErrorExp();
return new ErrorExp();
}
return e;
return reorderSettingAAElem(sc);
}

/************************************************************/
Expand Down Expand Up @@ -11051,6 +11048,9 @@ Expression *PowAssignExp::semantic(Scope *sc)
else
{
e1 = e1->modifiableLvalue(sc, e1);

e = reorderSettingAAElem(sc);
if (e != this) return e;
}

if ( (e1->type->isintegral() || e1->type->isfloating()) &&
Expand Down Expand Up @@ -12661,3 +12661,63 @@ SliceExp *resolveOpDollar(Scope *sc, SliceExp *se)
sc = sc->pop();
return se;
}

Expression *BinExp::reorderSettingAAElem(Scope *sc)
{
if (this->e1->op != TOKindex)
return this;
IndexExp *ie = (IndexExp *)e1;
Type *t1 = ie->e1->type->toBasetype();
if (t1->ty != Taarray)
return this;

/* Check recursive conversion */
VarDeclaration *var;
bool isrefvar = (e2->op == TOKvar &&
(var = ((VarExp *)e2)->var->isVarDeclaration()) != NULL &&
(var->storage_class & STCref));
if (isrefvar)
return this;

/* Fix evaluation order of setting AA element. (Bugzilla 3825)
* Rewrite:
* aa[key] op= val;
* as:
* ref __aatmp = aa;
* ref __aakey = key;
* ref __aaval = val;
* __aatmp[__aakey] op= __aaval; // assignment
*/
Expression *ec = NULL;
if (ie->e1->hasSideEffect())
{
Identifier *id = Lexer::uniqueId("__aatmp");
VarDeclaration *vd = new VarDeclaration(ie->e1->loc, ie->e1->type, id, new ExpInitializer(ie->e1->loc, ie->e1));
vd->storage_class |= STCref | STCforeach;
Expression *de = new DeclarationExp(ie->e1->loc, vd);

ec = de;
ie->e1 = new VarExp(ie->e1->loc, vd);
}
if (ie->e2->hasSideEffect())
{
Identifier *id = Lexer::uniqueId("__aakey");
VarDeclaration *vd = new VarDeclaration(ie->e2->loc, ie->e2->type, id, new ExpInitializer(ie->e2->loc, ie->e2));
vd->storage_class |= STCref | STCforeach;
Expression *de = new DeclarationExp(ie->e2->loc, vd);

ec = ec ? new CommaExp(loc, ec, de) : de;
ie->e2 = new VarExp(ie->e2->loc, vd);
}
{
Identifier *id = Lexer::uniqueId("__aaval");
VarDeclaration *vd = new VarDeclaration(loc, this->e2->type, id, new ExpInitializer(this->e2->loc, this->e2));
vd->storage_class |= STCref | STCforeach;
Expression *de = new DeclarationExp(this->e2->loc, vd);

ec = ec ? new CommaExp(loc, ec, de) : de;
this->e2 = new VarExp(this->e2->loc, vd);
}
ec = new CommaExp(loc, ec, this);
return ec->semantic(sc);
}
1 change: 1 addition & 0 deletions src/expression.h
View file
Expand Up @@ -819,6 +819,7 @@ struct BinExp : Expression

Expression *op_overload(Scope *sc);
Expression *compare_overload(Scope *sc, Identifier *id);
Expression *reorderSettingAAElem(Scope *sc);

elem *toElemBin(IRState *irs, int op);
};
Expand Down
118 changes: 118 additions & 0 deletions test/runnable/testaa2.d
View file
Expand Up @@ -105,13 +105,131 @@ void test4523()
static assert({ foo4523(); return true; }());
}

/************************************************/
// 3825

import std.math; // necessary for ^^=
void test3825()
{
// Check for RangeError is thrown
bool thrown(T)(lazy T cond)
{
import core.exception;
bool f = false;
try {
cond();
} catch (RangeError e) { f = true; }
return f;
}

int[int] aax;
int[][int] aay;

aax = null, aay = null;
assert(thrown(aax[0]));
assert(thrown(aax[0] = aax[0])); // rhs throws
assert(thrown(aax[0] += aax[0])); // rhs throws
assert(thrown(aax[0] ^^= aax[0])); // rhs throws
assert(thrown(aay[0] ~= aay[0])); // rhs throws
aax = null; aax[0] = 1; assert(aax[0] == 1); // setting aax[0] is OK
aax = null; aax[0] += 1; assert(aax[0] == +1); // setting aax[0] to 0 and modify it is OK
aax = null; aax[0] ^^= 1; assert(aax[0] == 0); // setting aax[0] to 0 and modify it is OK
aay = null; aay[0] ~= []; assert(aay[0] == []); // setting aay[0] to 0 and modify it is OK
aax = null; ++aax[0]; assert(aax[0] == +1); // setting aax[0] to 0 and modify it is OK
aax = null; --aax[0]; assert(aax[0] == -1); // setting aax[0] to 0 and modify it is OK

aax = [0:0], aay = [0:null];
assert(thrown(aax[aax[1]] = 1)); // accessing aax[1] in key part throws
assert(thrown(aax[aax[1]] += 1)); // accessing aax[1] in key part throws
assert(thrown(aax[aax[1]] ^^= 1)); // accessing aax[1] in key part throws
assert(thrown(aay[aax[1]] ~= [])); // accessing aax[1] in key part throws

//assert(thrown(aax[( aax[1], 0)] = 0));
/* accessing aax[1] in key part, why doesn't throw?
* Because, in aax[(aax[1], 0)], aax[1] is in lhs of comma expression, and is treated
* it has no side effect. Then optimizer eliminate it completely, and
* whole expression succeed to run in runtime. */
int n = 0;
assert(thrown(aax[(n=aax[1], 0)] = 0)); // accessing aax[1] in key part, throws OK

// This works as expected.
int[int][int] aaa;
aaa[0][0] = 0; assert(aaa[0][0] == 0); // setting aaa[0][0] is OK

// real test cases
void bug3825()
{
string[] words = ["how", "are", "you", "are"];

int[string] aa1;
foreach (w; words)
aa1[w] = ((w in aa1) ? (aa1[w] + 1) : 2);
//writeln(aa1); // Prints: [how:1,you:1,are:2]

int[string] aa2;
foreach (w; words)
if (w in aa2)
aa2[w]++;
else
aa2[w] = 2;
//writeln(aa2); // Prints: [how:2,you:2,are:3]

assert(aa1 == aa2);
assert(aa1 == ["how":2, "you":2, "are":3]);
assert(aa2 == ["how":2, "you":2, "are":3]);
}
void bug5021()
{
int func()
{
throw new Exception("It's an exception.");
}

int[string] arr;
try
{
arr["hello"] = func();
}
catch(Exception e)
{
}
assert(arr.length == 0);
}
void bug7914()
{
size_t[ubyte] aa;
aa[0] = aa.length;
assert(aa[0] == 0);
}
void bug8070()
{
Object[string] arr;

class A
{
this()
{
// at this point:
assert("x" !in arr);
}
}

arr["x"] = new A();
}
bug3825();
bug5021();
bug7914();
bug8070();
}

/************************************************/

int main()
{
testaa();
test1899();
test4523();
test3825();

printf("Success\n");
return 0;
Expand Down

0 comments on commit 4af7946

Please sign in to comment.