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.

Sign up for GitHub

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

Jump to bottom

Fix Issue 20415 - ice in dwarfeh and when optimizations are enabled #10630

Closed
wants to merge 1 commit into from
Closed

Fix Issue 20415 - ice in dwarfeh and when optimizations are enabled #10630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 4 additions & 2 deletions src/dmd/expression.d
View file
Open in desktop
Expand Up @@ -6527,7 +6527,9 @@ extern (C++) final class CondExp : BinExp

if (v.needsScopeDtor())
{
if (!vcond)
// a temporary is created for the condition
// only if it's not an integral exp
if (!vcond && !ce.econd.isIntegerExp())
Copy link
Member

Choose a reason for hiding this comment

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

This may be a bit simplistic. The optimizer is pretty smart, it is able to optimize array literal indexing, enums, const scalar variables and similar things.
The following will still segfault

bool ct() { return 1+1 == 5; }
void main()
{
    enum arr = [ct()];
    auto a = arr[1*0] ? B().p : null;
}
struct B
{
    void* p;
    ~this() {}
}

Since we don't want to run the optimizer here just yet you can just declare the vcond variable as const or immutable bool then the optimizer later on should be able to const fold it and eliminate the dead branch.
See

dmd/src/dmd/optimize.d

Lines 76 to 81 in ce96179

if (v.type &&
(v.isConst() || v.isImmutable() || v.storage_class & STC.manifest))
{
Type tb = v.type.toBasetype();
if (v.storage_class & STC.manifest ||
tb.isscalar() ||

{
vcond = copyToTemp(STC.volatile_, "__cond", ce.econd);
vcond.dsymbolSemantic(sc);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
vcond.dsymbolSemantic(sc);
vcond.type = Type.tbool.immutableOf();
vcond.dsymbolSemantic(sc);

Expand All @@ -6540,7 +6542,7 @@ extern (C++) final class CondExp : BinExp
}

//printf("\t++v = %s, v.edtor = %s\n", v.toChars(), v.edtor.toChars());
Expression ve = new VarExp(vcond.loc, vcond);
Expression ve = vcond ? new VarExp(vcond.loc, vcond) : ce.econd;
if (isThen)
v.edtor = new LogicalExp(v.edtor.loc, TOK.andAnd, ve, v.edtor);
else
Expand Down
14 changes: 14 additions & 0 deletions test/compilable/test20415.d
View file
Open in desktop
@@ -0,0 +1,14 @@
// https://issues.dlang.org/show_bug.cgi?id=20415
// REQUIRED_ARGS: -c -O

void t()
{
auto a = false ? B().p : null;
}

struct B
{
void* p;
~this() {}
}