[clang] Improve -Wdeclaration-after-statement

[melver]

Summary

With 118f966b46cf, Clang matches GCC's behaviour and allows enabling
-Wdeclaration-after-statement with C99 and later.

However, the check for mixing declarations and code is not a constant time
algorithm, and therefore should be guarded with Diags.isIgnored().

Furthermore, improve test coverage with: non-pedantic C89 with the
warning; C11 with the warning; and when using -Wall.

Finally, mention the changed behaviour in ReleaseNotes.rst.

[melver]

Hello,

A quick review would be much appreciated. I intend to submit this before Feb 1 so it may be included in Clang 14.

In case I have not added the appropriate reviewer(s) of the modified code, please let me know.

Thank you.

[aaron.ballman]

Can you also add a release note about the change?

clang/test/Sema/warn-mixed-decls-code.c

1–3 ↗

(On Diff #399688)

7 ↗

(On Diff #399688)

9–11 ↗

(On Diff #399688)

18–21 ↗

(On Diff #399688)

[melver]

Looks like https://reviews.llvm.org/D114787 does the same thing and was submitted a week ago.

I'll try to integrate the additional testing you suggested and change this patch to improve the implementation to not do the checking if not required as it doesn't look entirely cheap.

[aaron.ballman]

In D117232#3258309, @melver wrote:

Looks like https://reviews.llvm.org/D114787 does the same thing and was submitted a week ago.

I'll try to integrate the additional testing you suggested and change this patch to improve the implementation to not do the checking if not required as it doesn't look entirely cheap.

I *knew* this seemed super familiar! Sorry that my search through my email missed that. :-D Yes, please rebase on trunk and add the improved isIgnored() check. Thanks!

[melver]

Rebase and rework to include improvements over already submitted code.

[aaron.ballman]

LGTM aside from a small nit with the release notes.

clang/docs/ReleaseNotes.rst

61

[melver]

Use `..` in ReleaseNotes.rst.

Thanks!

clang/docs/ReleaseNotes.rst

 
 - -Wbitwise-instead-of-logical (part of -Wbool-operation) warns about use of bitwise operators with boolean operands which have side effects.
 
 - Added diagnostic groups to control diagnostics for attribute extensions by
   adding groups ``-Wc++N-attribute-extensions`` (where ``N`` is the standard
   release being diagnosed against). These new groups are automatically implied
   when passing ``-Wc++N-extensions``. Resolves PR33518.
 
+- Support ``-Wdeclaration-after-statement`` with C99 and later standards, and
+  not just C89, matching GCC's behaviour. A notable usecase is supporting style
+  guides that forbid mixing declarations and code, but want to move to newer C
+  standards.
+
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 
 - Maximum _ExtInt size was decreased from 16,777,215 bits to 8,388,608 bits.
   Motivation for this was discussed in PR51829.
 - Configuration file syntax extended with ``<CFGDIR>`` token. This expands to
   the base path of the current config file. See :ref:`configuration-files` for
   details.

clang/lib/Sema/SemaStmt.cpp

 
 StmtResult Sema::ActOnCompoundStmt(SourceLocation L, SourceLocation R,
                                    ArrayRef<Stmt *> Elts, bool isStmtExpr) {
   const unsigned NumElts = Elts.size();
 
   // If we're in C mode, check that we don't have any decls after stmts.  If
   // so, emit an extension diagnostic in C89 and potentially a warning in later
   // versions.
-  if (!getLangOpts().CPlusPlus) {
+  const unsigned MixedDeclsCodeID = getLangOpts().C99
+                                        ? diag::warn_mixed_decls_code
+                                        : diag::ext_mixed_decls_code;
+  if (!getLangOpts().CPlusPlus && !Diags.isIgnored(MixedDeclsCodeID, L)) {
     // Note that __extension__ can be around a decl.
     unsigned i = 0;
     // Skip over all declarations.
     for (; i != NumElts && isa<DeclStmt>(Elts[i]); ++i)
       /*empty*/;
 
     // We found the end of the list or a statement.  Scan for another declstmt.
     for (; i != NumElts && !isa<DeclStmt>(Elts[i]); ++i)
       /*empty*/;
 
     if (i != NumElts) {
       Decl *D = *cast<DeclStmt>(Elts[i])->decl_begin();
-      Diag(D->getLocation(), !getLangOpts().C99 ? diag::ext_mixed_decls_code
-                                                : diag::warn_mixed_decls_code);
+      Diag(D->getLocation(), MixedDeclsCodeID);
     }
   }
 
   // Check for suspicious empty body (null statement) in `for' and `while'
   // statements.  Don't do anything for template instantiations, this just adds
   // noise.
   if (NumElts != 0 && !CurrentInstantiationScope &&
       getCurCompoundScope().HasEmptyLoopBodies) {

clang/test/Sema/warn-mixed-decls.c

 /* RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -pedantic %s
  */
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wdeclaration-after-statement %s
+ */
 /* RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wdeclaration-after-statement %s
  */
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -Wdeclaration-after-statement %s
+ */
 
 /* Should not emit diagnostic when not pedantic, not enabled or in C++ Code*/
 /* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c89 %s
  */
 /* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c99 %s
  */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c89 -Wall %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c99 -Wall -pedantic %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c11 -Wall -pedantic %s
+ */
 /* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ %s
  */
 /* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ -Wdeclaration-after-statement %s
  */
 
 /* none-no-diagnostics */
 
 int foo(int i)