Skip to content

Commit

Permalink
Merge pull request #11005 from kinke/context
Browse files Browse the repository at this point in the history 
[stable] Fix misc. issues wrt. temporaries for -checkaction=context lowering
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
  • Loading branch information
@dlang-bot
dlang-bot committed Apr 8, 2020
2 parents d96d44e + f4dc07c commit cbe2eee
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 45 deletions.
84 changes: 43 additions & 41 deletions src/dmd/expressionsem.d
View file
Expand Up @@ -5809,20 +5809,8 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
printf("AssertExp::semantic('%s')\n", exp.toChars());
}

// save expression as a string before any semantic expansion
// if -checkaction=context is enabled an no message exists
const generateMsg = !exp.msg && global.params.checkAction == CHECKACTION.context;
auto assertExpMsg = generateMsg ? exp.toChars() : null;

if (Expression ex = unaSemantic(exp, sc))
{
result = ex;
return;
}
exp.e1 = resolveProperties(sc, exp.e1);
// BUG: see if we can do compile time elimination of the Assert
exp.e1 = exp.e1.optimize(WANTvalue);
exp.e1 = exp.e1.toBoolean(sc);
Expression temporariesPrefix;

if (generateMsg)
// no message - use assert expression as msg
Expand All @@ -5840,29 +5828,38 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
effects as in https://issues.dlang.org/show_bug.cgi?id=20114
Params:
op = an expression which may require a temporary and will be
replaced by `(auto tmp = op, tmp)` if necessary
op = an expression which may require a temporary (added to
`temporariesPrefix`: `auto tmp = op`) and will be replaced
by `tmp` if necessary
Returns: `op` or `tmp` for subsequent access to the possibly promoted operand
Returns: (possibly replaced) `op`
*/
Expression maybePromoteToTmp(ref Expression op)
{
op = op.expressionSemantic(sc);
op = resolveProperties(sc, op);
if (op.hasSideEffect)
{
const stc = STC.exptemp | (op.isLvalue() ? STC.ref_ : STC.rvalue);
const stc = op.isLvalue() ? STC.ref_ : 0;
auto tmp = copyToTemp(stc, "__assertOp", op);
tmp.dsymbolSemantic(sc);

auto decl = new DeclarationExp(op.loc, tmp);
auto var = new VarExp(op.loc, tmp);
auto comb = Expression.combine(decl, var);
op = comb.expressionSemantic(sc);
temporariesPrefix = Expression.combine(temporariesPrefix, decl);

return var;
op = new VarExp(op.loc, tmp);
op = op.expressionSemantic(sc);
}
return op;
}

// if the assert condition is a mixin expression, try to compile it
if (auto ce = exp.e1.isCompileExp())
{
if (auto e1 = compileIt(ce))
exp.e1 = e1;
}

const tok = exp.e1.op;
bool isEqualsCallExpression;
if (tok == TOK.call)
Expand Down Expand Up @@ -5898,31 +5895,20 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
auto callExp = cast(CallExp) exp.e1;
auto args = callExp.arguments;

// template args
static immutable compMsg = "==";
Expression comp = new StringExp(loc, compMsg);
comp = comp.expressionSemantic(sc);
(*tiargs)[0] = comp;

// structs with opEquals get rewritten to a DotVarExp:
// a.opEquals(b)
// https://issues.dlang.org/show_bug.cgi?id=20100
if (args.length == 1)
{
auto dv = callExp.e1.isDotVarExp();
assert(dv);
(*tiargs)[1] = dv.e1.type;
(*tiargs)[2] = (*args)[0].type;

// runtime args
(*es)[0] = maybePromoteToTmp(dv.e1);
(*es)[1] = maybePromoteToTmp((*args)[0]);
}
else
{
(*tiargs)[1] = (*args)[0].type;
(*tiargs)[2] = (*args)[1].type;

// runtime args
(*es)[0] = maybePromoteToTmp((*args)[0]);
(*es)[1] = maybePromoteToTmp((*args)[1]);
Expand All @@ -5932,18 +5918,18 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
{
auto binExp = cast(EqualExp) exp.e1;

// template args
Expression comp = new StringExp(loc, Token.toString(exp.e1.op));
comp = comp.expressionSemantic(sc);
(*tiargs)[0] = comp;
(*tiargs)[1] = binExp.e1.type;
(*tiargs)[2] = binExp.e2.type;

// runtime args
(*es)[0] = maybePromoteToTmp(binExp.e1);
(*es)[1] = maybePromoteToTmp(binExp.e2);
}

// template args
Expression comp = new StringExp(loc, isEqualsCallExpression ? "==" : Token.toString(exp.e1.op));
comp = comp.expressionSemantic(sc);
(*tiargs)[0] = comp;
(*tiargs)[1] = (*es)[0].type;
(*tiargs)[2] = (*es)[1].type;

Expression __assertFail = new IdentifierExp(exp.loc, Id.empty);
auto assertFail = new DotIdExp(loc, __assertFail, Id.object);

Expand All @@ -5954,10 +5940,22 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
else
{
OutBuffer buf;
buf.printf("%s failed", assertExpMsg);
buf.printf("%s failed", exp.toChars());
exp.msg = new StringExp(Loc.initial, buf.extractSlice());
}
}

if (Expression ex = unaSemantic(exp, sc))
{
result = ex;
return;
}

exp.e1 = resolveProperties(sc, exp.e1);
// BUG: see if we can do compile time elimination of the Assert
exp.e1 = exp.e1.optimize(WANTvalue);
exp.e1 = exp.e1.toBoolean(sc);

if (exp.msg)
{
exp.msg = expressionSemantic(exp.msg, sc);
Expand Down Expand Up @@ -5999,8 +5997,12 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
return;
}
}

exp.type = Type.tvoid;
result = exp;

result = !temporariesPrefix
? exp
: Expression.combine(temporariesPrefix, exp).expressionSemantic(sc);
}

override void visit(DotIdExp exp)
Expand Down
62 changes: 58 additions & 4 deletions test/runnable/testassert.d
View file
Expand Up @@ -75,6 +75,8 @@ void test20114()
assert(c == "1 != 0");
}

version (DigitalMars) version (Win64) version = DMD_Win64;

void test20375() @safe
{
static struct RefCounted
Expand All @@ -87,6 +89,7 @@ void test20375() @safe
}

static int instances;
static int postblits;

this(bool) @safe
{
Expand All @@ -96,10 +99,14 @@ void test20375() @safe
this(this) @safe
{
instances++;
postblits++;
}

~this() @safe
{
// make the dtor non-nothrow (we are tracking clean-ups during AssertError unwinding)
if (postblits > 100)
throw new Exception("");
assert(instances > 0);
instances--;
}
Expand All @@ -112,19 +119,54 @@ void test20375() @safe

{
auto a = RefCounted.create();
assert(a == RefCounted.create());
RefCounted.instances++; // we're about to construct an instance below, increment manually
assert(a == RefCounted()); // both operands are pure expressions => no temporaries
}

assert(RefCounted.instances == 0);
assert(RefCounted.postblits == 0);

{
auto a = RefCounted.create();
const msg = getMessage(assert(a != RefCounted.create()));
// assert(msg == "RefCounted() == RefCounted()"); // Currently not formatted
assert(msg == "assert(a != RefCounted.create()) failed");
assert(a == RefCounted.create()); // impure rhs is promoted to a temporary lvalue => copy for a.opEquals(rhs)
}

assert(RefCounted.instances == 0);
assert(RefCounted.postblits == 1);
RefCounted.postblits = 0;

{
const msg = getMessage(assert(RefCounted.create() != RefCounted.create())); // both operands promoted to temporary lvalues
assert(msg == "RefCounted() == RefCounted()");
}

version (DMD_Win64) // FIXME: temporaries apparently not destructed when unwinding via AssertError
{
assert(RefCounted.instances >= 0 && RefCounted.instances <= 2);
RefCounted.instances = 0;
}
else
assert(RefCounted.instances == 0);
assert(RefCounted.postblits == 1);
RefCounted.postblits = 0;

static int numGetLvalImpureCalls = 0;
ref RefCounted getLvalImpure() @trusted
{
numGetLvalImpureCalls++;
__gshared lval = RefCounted(); // not incrementing RefCounted.instances
return lval;
}

{
const msg = getMessage(assert(getLvalImpure() != getLvalImpure())); // both operands promoted to ref temporaries
assert(msg == "RefCounted() == RefCounted()");
}

assert(numGetLvalImpureCalls == 2);
assert(RefCounted.instances == 0);
assert(RefCounted.postblits == 1);
RefCounted.postblits = 0;
}

string getMessage(T)(lazy T expr) @trusted
Expand All @@ -140,10 +182,22 @@ string getMessage(T)(lazy T expr) @trusted
}
}

void testMixinExpression() @safe
{
static struct S
{
bool opEquals(S) @safe { return true; }
}

const msg = getMessage(assert(mixin("S() != S()")));
assert(msg == "S() == S()");
}

void main()
{
test8765();
test9255();
test20114();
test20375();
testMixinExpression();
}

0 comments on commit cbe2eee

Please sign in to comment.