[PATCH] D40445: [C++17] Allow an empty expression in an if init statement

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Parse/ParseExprCXX.cpp:1760
+    if (InitStmt && TryConsumeToken(tok::semi)) {
+      WarnOnInit();
+      return ParseCXXCondition(nullptr, Loc, CK);
----------------
Please create a NullStmt here (`Actions.ActOnNullStmt`), rather than producing an AST that's indistinguishable from one where the init statement was omitted.


================
Comment at: lib/Parse/ParseExprCXX.cpp:1780-1783
     Diag(Tok.getLocation(), getLangOpts().CPlusPlus17
                                 ? diag::warn_cxx14_compat_init_statement
                                 : diag::ext_init_statement)
         << (CK == Sema::ConditionKind::Switch);
----------------
Use `WarnOnInit()` here.


================
Comment at: test/CXX/stmt.stmt/stmt.select/p3.cpp:70-71
+
+// TODO: This is needed because clang can't seem to diagnose invalid syntax after the
+// last loop above. It would be nice to remove this.
+void whileInitStatement2() {
----------------
I think error recovery thought that a `)` was omitted before the `;`, so it imagined we had:

 ```
  while (Var + Var); Var--) {}
```

Then it thought that a `;` was omitted before the `)`, so it imagined we had:

 ```
  while (Var + Var); Var--; ) {}
```

Then it saw a statement beginning with a `)`, and decided to skip to the next statement, which would skip past the `)`, the `{`, the `}`, and whatever statement you put next in the function.

Maybe we should parse as if an //init-statement// were permitted on a `while` loop (and produce an error after the fact if there's one present). But that should be done in a separate patch if you're interested in working on it.


https://reviews.llvm.org/D40445