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 17335 - Function calls in conjunctions do not short circuit… #6713

Merged
merged 1 commit into from
Apr 21, 2017

Conversation

WalterBright
Copy link
Member

… when evaluated during compilation

Also fix https://issues.dlang.org/show_bug.cgi?id=17334

@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
17335 Function calls in conjunctions do not short circuit when evaluated during compilation

Copy link
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

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

See comment.

result = evalStaticCondition(sc, exp, leg, errors);
return !errors && result;
}

Copy link
Member

Choose a reason for hiding this comment

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

Nice, but what if the user introduces a redundant set of parens?

static if ((cond1 && cond2)) ...

Then the top-level node is a TOKparen or whatevs, and would hide the conjunction inside. Is that the case? If so, it needs to be handled too.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are no paren expressions in the AST. (a+b) is the same AST as a+b. Parens are for grouping.

static if (!alwaysFalse() ? 1 : f == 1)
{
}
}
Copy link
Member

Choose a reason for hiding this comment

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

So the extra test here would be all of the above in one level of parens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants