Skip to content
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

Issue 3825 - AAs entries are default initialized before the new value is evaluated #1465

Merged
merged 1 commit into from Mar 4, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 69 additions & 9 deletions src/expression.c
Expand Up @@ -6424,8 +6424,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 @@ -6491,7 +6490,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 @@ -10994,7 +10994,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 @@ -11079,7 +11079,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 @@ -11088,7 +11087,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 @@ -11098,7 +11096,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 @@ -11108,9 +11105,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 @@ -11217,6 +11214,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 @@ -12874,3 +12874,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
Expand Up @@ -820,6 +820,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
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