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

Fix Issue 519 - Invariant not called from autogenerated class/struct constructor/destructor #10022

Closed
wants to merge 1 commit into from
Closed
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
6 changes: 6 additions & 0 deletions src/dmd/clone.d
Expand Up @@ -970,6 +970,12 @@ DtorDeclaration buildDtor(AggregateDeclaration ad, Scope* sc)
switch (ad.dtors.dim)
{
case 0:
/* If running invariant, need a destructor to hang it on,
* but only do for root modules, as ones in the library may not
* have been compiled with useInvariants
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contracts (and possibly invariants) should really be generated and called by the caller, just like a template.

*/
if (global.params.useInvariants && ad.inv && sc._module.isRoot())
goto default;
break;

case 1:
Expand Down
14 changes: 6 additions & 8 deletions src/dmd/dsymbolsem.d
Expand Up @@ -4611,6 +4611,9 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
*/
sd.aggNew = cast(NewDeclaration)sd.search(Loc.initial, Id.classNew);
sd.aggDelete = cast(DeleteDeclaration)sd.search(Loc.initial, Id.classDelete);
sd.inv = buildInv(sd, sc2);
if (sd.inv)
reinforceInvariant(sd, sc2);

// Look for the constructor
sd.ctor = sd.searchCtor();
Expand All @@ -4630,10 +4633,6 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
sd.xhash = buildXtoHash(sd, sc2);
}

sd.inv = buildInv(sd, sc2);
if (sd.inv)
reinforceInvariant(sd, sc2);

Module.dprogress++;
sd.semanticRun = PASS.semanticdone;
//printf("-StructDeclaration::semantic(this=%p, '%s')\n", sd, sd.toChars());
Expand Down Expand Up @@ -5207,6 +5206,9 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
// Can be in base class
cldec.aggNew = cast(NewDeclaration)cldec.search(Loc.initial, Id.classNew);
cldec.aggDelete = cast(DeleteDeclaration)cldec.search(Loc.initial, Id.classDelete);
cldec.inv = buildInv(cldec, sc2);
if (cldec.inv)
reinforceInvariant(cldec, sc2);

// Look for the constructor
cldec.ctor = cldec.searchCtor();
Expand Down Expand Up @@ -5280,10 +5282,6 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
cldec.error(f.loc, "identity assignment operator overload is illegal");
}

cldec.inv = buildInv(cldec, sc2);
if (cldec.inv)
reinforceInvariant(cldec, sc2);

Module.dprogress++;
cldec.semanticRun = PASS.semanticdone;
//printf("-ClassDeclaration.dsymbolSemantic(%s), type = %p\n", toChars(), type);
Expand Down
12 changes: 12 additions & 0 deletions src/dmd/e2ir.d
Expand Up @@ -5814,6 +5814,18 @@ private elem *toElemStructLit(StructLiteralExp sle, IRState *irs, TOK op, Symbol
e = el_combine(e, e2);
}

if (global.params.useInvariants && sle.sd.inv)
{
/* Should only call invariant if this is not a default initialization of the struct.
*/
if (!sle.isDefaultStructInitializerExp)
{
FuncDeclaration inv = sle.sd.inv;
elem *einv = callfunc(sle.loc, irs, 1, inv.type.nextOf(), el_ptr(stmp), sle.sd.type.pointerTo(), inv, inv.type, null, null);
e = el_combine(e, einv);
}
}

elem *ev = el_var(stmp);
ev.ET = Type_toCtype(sle.sd.type);
e = el_combine(e, ev);
Expand Down
3 changes: 3 additions & 0 deletions src/dmd/expression.d
Expand Up @@ -3074,6 +3074,7 @@ extern (C++) final class StructLiteralExp : Expression
int stageflags;

bool useStaticInit; /// if this is true, use the StructDeclaration's init symbol
bool isDefaultStructInitializerExp;
OwnedBy ownedByCtfe = OwnedBy.code;

extern (D) this(const ref Loc loc, StructDeclaration sd, Expressions* elements, Type stype = null)
Expand All @@ -3083,6 +3084,8 @@ extern (C++) final class StructLiteralExp : Expression
if (!elements)
elements = new Expressions();
this.elements = elements;
if (elements.dim == 0)
isDefaultStructInitializerExp = true;
this.stype = stype;
this.origin = this;
//printf("StructLiteralExp::StructLiteralExp(%s)\n", toChars());
Expand Down
1 change: 1 addition & 0 deletions src/dmd/expression.h
Expand Up @@ -464,6 +464,7 @@ class StructLiteralExp : public Expression
int stageflags;

bool useStaticInit; // if this is true, use the StructDeclaration's init symbol
bool isDefaultStructInitializerExp;
OwnedBy ownedByCtfe;

static StructLiteralExp *create(Loc loc, StructDeclaration *sd, void *elements, Type *stype = NULL);
Expand Down
2 changes: 2 additions & 0 deletions test/runnable/test19731.d
Expand Up @@ -45,6 +45,8 @@ void main()
}
catch(AssertError)
{
foo.obj_ = new Object();
foo2.obj_ = new Object();
return;
}
assert(0);
Expand Down
4 changes: 3 additions & 1 deletion test/runnable/testdstress.d
Expand Up @@ -815,10 +815,12 @@ class Parent40{

invariant()
{
assert(!checked40);
if(!checked40)
{
checked40=true;
// even number
assert((x&1u)==0);
}
}
}

Expand Down
80 changes: 80 additions & 0 deletions test/runnable/testinvariant.d
Expand Up @@ -25,6 +25,85 @@ int testinvariant()
return 0;
}

// https://issues.dlang.org/show_bug.cgi?id=519
class C519
{
invariant
{
printf("C519.invariant\n");
++x;
}
__gshared int x;
}

struct S519
{
invariant()
{
printf("S519.invariant\n");
++x;
}
__gshared int x;
}

struct S519c
{
invariant()
{
printf("S519c.invariant\n");
++x;
}
__gshared int x;
int y;
}

void test519()
{
//printf("test1\n");
{
auto foo = new C519();
assert(C519.x == 0);
printf("lifetime of foo\n");
destroy(foo);
assert(C519.x == 1);
}
{
scope auto foo = new C519();
assert(C519.x == 1);
printf("lifetime of foo\n");
}
assert(C519.x == 2);

//printf("test2\n");
{
auto foo = new S519();
printf("%d\n", S519.x);
assert(S519.x == 0);
printf("lifetime of foo\n");
destroy(*foo);
assert(S519.x == 1);
}
{
auto foo = S519();
assert(S519.x == 1);
printf("lifetime of foo\n");
}
assert(S519.x == 2);

//printf("test3\n");
{
auto foo = new S519c(1);
assert(S519c.x == 1);
printf("lifetime of foo\n");
}
{
auto foo = S519c(1);
assert(S519c.x == 2);
printf("lifetime of foo\n");
}
assert(S519c.x == 3);
}

/***************************************************/
// https://issues.dlang.org/show_bug.cgi?id=6453

Expand Down Expand Up @@ -182,6 +261,7 @@ void test13147()
void main()
{
testinvariant();
test519();
test6453();
test13113();
test13147();
Expand Down