Skip to content

fix Issue 12486 - Function returning struct isn't called if enum of its result is accessed #8013

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

Merged
dlang-bot merged 2 commits into from
Apr 8, 2018
Merged

fix Issue 12486 - Function returning struct isn't called if enum of its result is accessed #8013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a6d7c4
Add test from bug #12486
FeepingCreature Mar 12, 2018
775ee59
Fix #12486: avoid discarding expression when accessing an enum member…
FeepingCreature Mar 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/dmd/typesem.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3223,6 +3223,10 @@ private extern(C++) final class DotExpVisitor : Visitor
}
checkAccess(e.loc, sc, null, v);
Expression ve = new VarExp(e.loc, v);
if (!isTrivialExp(e))
Copy link
Member

@ibuclaw ibuclaw Apr 7, 2018

Choose a reason for hiding this comment

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

@FeepingCreature - you could also use hasSideEffects(e) (or e.hasSideEffects() if you prefer UFCS). When having an initial glance at the implementation, it didn't seem like it was a strong enough guarantee, but having a closer look, only strongly pure functions would have their calls omitted here. Which is harmless.

Copy link
Contributor Author

@FeepingCreature FeepingCreature Apr 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I can tell, pure void foo() { while (true) { } } is strongly pure, but must under no circumstances be optimized away. That exact case was what triggered this bug in the first place.

Pure expressions can be reduced from two to one, but only trivial expressions can be reduced from one to zero.

Copy link
Member

@ibuclaw ibuclaw Apr 8, 2018

Choose a reason for hiding this comment

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

Ah, bugger those looping pure functions. :-)

Yep, makes sense, and LGTM.

Copy link
Member

@ibuclaw ibuclaw Apr 8, 2018

Choose a reason for hiding this comment

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

@FeepingCreature - For brownie points, you can add a comment with a link to the bugzilla reference.

{
ve = new CommaExp(e.loc, e, ve);
}
ve = ve.expressionSemantic(sc);
result = ve;
return;
Expand Down
18 changes: 18 additions & 0 deletions test/runnable/test12486.d
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module test12486;

struct S { enum a = 1; } // or `const` but not for all types

S f(ref int i)
{
++i;
return S();
}

void main()
{
int i = 2;
assert(f(i).a == 1);
// ensure that f(i) was actually called, even though
// a is a statically known property of the returned type
assert(i == 3);
}