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