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 #20645 - implement C++ style init-statement for if #20653

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion compiler/src/dmd/cparse.d
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ final class CParser(AST) : Parser!AST
else
elsebody = null;
if (condition && ifbody)
s = new AST.IfStatement(loc, null, condition, ifbody, elsebody, token.loc);
s = new AST.IfStatement(loc, null, condition, null, ifbody, elsebody, token.loc);
else
s = null; // don't propagate parsing errors
break;
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dmd/dsymbolsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -2700,7 +2700,7 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
}
IntegerExp cmp = isDestructor ? IntegerExp.literal!0 : IntegerExp.literal!1;
e = new EqualExp(EXP.notEqual, Loc.initial, e, cmp);
s = new IfStatement(Loc.initial, null, e, new ReturnStatement(Loc.initial, null), null, Loc.initial);
s = new IfStatement(Loc.initial, null, e, null, new ReturnStatement(Loc.initial, null), null, Loc.initial);

sa.push(s);
if (sd.fbody)
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dmd/inline.d
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ public:

static if (asStatements)
{
result = new IfStatement(s.loc, s.param, econd, ifbody, elsebody, s.endloc);
result = new IfStatement(s.loc, s.param, econd, s._init, ifbody, elsebody, s.endloc);
}
else
{
Expand Down Expand Up @@ -1034,7 +1034,7 @@ public:
return null;
auto ifbody = !s1 ? new ExpStatement(e.e1.loc, e.e1) : s1;
auto elsebody = !s2 ? new ExpStatement(e.e2.loc, e.e2) : s2;
return new IfStatement(exp.loc, null, e.econd, ifbody, elsebody, exp.loc);
return new IfStatement(exp.loc, null, e.econd, null, ifbody, elsebody, exp.loc);
}
if (auto e = exp.isCommaExp())
{
Expand Down
14 changes: 12 additions & 2 deletions compiler/src/dmd/parse.d
Original file line number Diff line number Diff line change
Expand Up @@ -6246,10 +6246,20 @@
}
case TOK.if_:
{
AST.Statement _init;
nextToken();
check(TOK.leftParenthesis);
auto param = parseAssignCondition();
auto condition = parseExpression();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Silly question: why does the old code do parseAssignCondition followed by parseExpression? Does parseAssignCondition not eat tokens?

Unlike with for which (assuming its there) unconditionally parses a statement before the first semicolon, I think here we would need to determine if there is a semicolon present before deciding wether or not to sparse a statement or an expression here.

Copy link
Contributor

Choose a reason for hiding this comment

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

parseAssignCondition only eats tokens if it sits on a storage class. If it does not, it just returns null. It also does not parse an initializer.

AFAIU, in if(auto x = e), parseAssignCondition parses the auto x = part, and parseExpression parses the e part.

// if ( _init ; condition )
if (token.value == TOK.semicolon)
{
const lookingForElseSave = lookingForElse;
lookingForElse = Loc.initial;
_init = parseStatement(0);
lookingForElse = lookingForElseSave;
nextToken();

Check warning on line 6260 in compiler/src/dmd/parse.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/parse.d#L6256-L6260

Added lines #L6256 - L6260 were not covered by tests
}
AST.Expression condition = parseExpression();
closeCondition("if", param, condition);

{
Expand All @@ -6268,7 +6278,7 @@
else
elsebody = null;
if (condition && ifbody)
s = new AST.IfStatement(loc, param, condition, ifbody, elsebody, token.loc);
s = new AST.IfStatement(loc, param, condition, _init, ifbody, elsebody, token.loc);
else
s = null; // don't propagate parsing errors
break;
Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dmd/statement.d
Original file line number Diff line number Diff line change
Expand Up @@ -937,16 +937,18 @@ extern (C++) final class IfStatement : Statement
{
Parameter param;
Expression condition;
Statement _init; // if ( init ; condition )
Statement ifbody;
Statement elsebody;
VarDeclaration match; // for MatchExpression results
Loc endloc; // location of closing curly bracket

extern (D) this(const ref Loc loc, Parameter param, Expression condition, Statement ifbody, Statement elsebody, Loc endloc) @safe
extern (D) this(const ref Loc loc, Parameter param, Expression condition, Statement _init, Statement ifbody, Statement elsebody, Loc endloc) @safe
{
super(loc, STMT.If);
this.param = param;
this.condition = condition;
this._init = _init;
this.ifbody = ifbody;
this.elsebody = elsebody;
this.endloc = endloc;
Expand All @@ -957,6 +959,7 @@ extern (C++) final class IfStatement : Statement
return new IfStatement(loc,
param ? param.syntaxCopy() : null,
condition.syntaxCopy(),
_init ? _init.syntaxCopy() : null,
ifbody ? ifbody.syntaxCopy() : null,
elsebody ? elsebody.syntaxCopy() : null,
endloc);
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dmd/statement.h
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ class IfStatement final : public Statement
public:
Parameter *param;
Expression *condition;
Statement* _init;
Statement *ifbody;
Statement *elsebody;
VarDeclaration *match; // for MatchExpression results
Expand Down
56 changes: 33 additions & 23 deletions compiler/src/dmd/statementsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@
* else
* { break; }
*/
_body = new IfStatement(ws.loc, ws.param, ws.condition, ws._body, new BreakStatement(ws.loc, null), ws.endloc);
_body = new IfStatement(ws.loc, ws.param, ws.condition, null, ws._body, new BreakStatement(ws.loc, null), ws.endloc);
cond = IntegerExp.createBool(true);
}
Statement s = new ForStatement(ws.loc, null, cond, null, _body, ws.endloc);
Expand Down Expand Up @@ -591,6 +591,30 @@
result = ds;
}

/* Rewrites:
* controlflow (auto v1 = i1, v2 = i2; condition [; increment]) { ... }
* to:
* { auto v1 = i1, v2 = i2; controlflow ([;] condition [; increment]) { ... } }
* then lowered to:
* auto v1 = i1;
* try {
* auto v2 = i2;
* try {
* controlflow ([;] condition [; increment]) { ... }
* } finally { v2.~this(); }
* } finally { v1.~this(); }
* where controlflow = `for` or `if`
*/
Statement expandInit(S)(S cfs)
{
auto ainit = new Statements();
ainit.push(cfs._init);
cfs._init = null;
ainit.push(cfs);
Statement s = new CompoundStatement(cfs.loc, ainit);
s = new ScopeStatement(cfs.loc, s, cfs.endloc);
return s.statementSemantic(sc);
}
void visitFor(ForStatement fs)
{
/* https://dlang.org/spec/statement.html#for-statement
Expand All @@ -599,26 +623,7 @@

if (fs._init)
{
/* Rewrite:
* for (auto v1 = i1, v2 = i2; condition; increment) { ... }
* to:
* { auto v1 = i1, v2 = i2; for (; condition; increment) { ... } }
* then lowered to:
* auto v1 = i1;
* try {
* auto v2 = i2;
* try {
* for (; condition; increment) { ... }
* } finally { v2.~this(); }
* } finally { v1.~this(); }
*/
auto ainit = new Statements();
ainit.push(fs._init);
fs._init = null;
ainit.push(fs);
Statement s = new CompoundStatement(fs.loc, ainit);
s = new ScopeStatement(fs.loc, s, fs.endloc);
s = s.statementSemantic(sc);
Statement s = expandInit(fs);
if (!s.isErrorStatement())
{
if (LabelStatement ls = checkLabeledLoop(sc, fs))
Expand Down Expand Up @@ -1637,6 +1642,11 @@
/* https://dlang.org/spec/statement.html#IfStatement
*/

if (ifs._init)
{
result = expandInit(ifs);
return;

Check warning on line 1648 in compiler/src/dmd/statementsem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/statementsem.d#L1647-L1648

Added lines #L1647 - L1648 were not covered by tests
}
// check in syntax level
ifs.condition = checkAssignmentAsCondition(ifs.condition, sc);

Expand Down Expand Up @@ -4161,7 +4171,7 @@

Expression ec = new IdentifierExp(loc, Id.ctfe);
ec = new NotExp(loc, ec);
Statement s = new IfStatement(loc, null, ec, new ExpStatement(loc, e), null, loc);
Statement s = new IfStatement(loc, null, ec, null, new ExpStatement(loc, e), null, loc);
c.handler = new TryFinallyStatement(loc, c.handler, s);
}

Expand Down Expand Up @@ -4266,7 +4276,7 @@

e = new VarExp(Loc.initial, v);
e = new NotExp(Loc.initial, e);
sfinally = new IfStatement(Loc.initial, null, e, s, null, Loc.initial);
sfinally = new IfStatement(Loc.initial, null, e, null, s, null, Loc.initial);

break;
}
Expand Down
12 changes: 12 additions & 0 deletions compiler/test/runnable/controlflow_init.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@


int main()
{
if (int a = 4; a)
{

}
else
assert(0);
return 0;
}
Loading