lebedev.ri added inline comments.

================
Comment at: lib/Parse/ParseStmt.cpp:237
+    SourceLocation SemiLocation = ConsumeToken();
+    if (!HasLeadingEmptyMacro && getCurScope()->isCompoundStmtScope() &&
+        !SemiLocation.isMacroID()) {
----------------
rsmith wrote:
> I'm a little concerned that checking whether the scope is a compound 
> statement isn't really checking the right thing -- what you care about is 
> whether the syntactic context is a compound statement, not which scope a 
> declaration at this level would be injected into. (Example of the difference: 
> back in the pre-standard times when `{ for (int x = 0; x < 10; ++x) //...` 
> injected `x` into the enclosing scope, the first substatement in that `for` 
> statement would be in a compound-statement scope, but we definitely shouldn't 
> warn on a stray `;` there.) If you moved this check into 
> `ParseCompoundStatementBody`, there'd be no risk of such problems.
Great idea, thank you!
Testcases added, that fixed them.


================
Comment at: test/Parser/extra-semi-resulting-in-nullstmt.cpp:59
+#if __cplusplus >= 201703L
+  if (; true) // OK
+    ;         // OK
----------------
rsmith wrote:
> It'd seem reasonable to warn on a null-statement as the *init-statement* of 
> an `if` / `switch` / range-based `for`.
If you so insist :)


Repository:
  rC Clang

https://reviews.llvm.org/D52695



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to