From 377748fd7bbf5798fe54a032e79d61c6feb86ccb Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Tue, 20 Nov 2018 18:59:05 +0000 Subject: [clang][Parse] Diagnose useless null statements / empty init-statements Summary: clang has `-Wextra-semi` (D43162), which is not dictated by the currently selected standard. While that is great, there is at least one more source of need-less semis - 'null statements'. Sometimes, they are needed: ``` for(int x = 0; continueToDoWork(x); x++) ; // Ugly code, but the semi is needed here. ``` But sometimes they are just there for no reason: ``` switch(X) { case 0: return -2345; case 5: return 0; default: return 42; }; // <- oops ;;;;;;;;;;; <- OOOOPS, still not diagnosed. Clearly this is junk. ``` Additionally: ``` if(; // <- empty init-statement true) ; switch (; // empty init-statement x) { ... } for (; // <- empty init-statement int y : S()) ; } As usual, things may or may not go sideways in the presence of macros. While evaluating this diag on my codebase of interest, it was unsurprisingly discovered that Google Test macros are *very* prone to this. And it seems many issues are deep within the GTest itself, not in the snippets passed from the codebase that uses GTest. So after some thought, i decided not do issue a diagnostic if the semi is within *any* macro, be it either from the normal header, or system header. Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]] Reviewers: rsmith, aaron.ballman, efriedma Reviewed By: aaron.ballman Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D52695 llvm-svn: 347339 --- clang/docs/ReleaseNotes.rst | 57 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) (limited to 'clang/docs/ReleaseNotes.rst') diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 5e6dfdf1c1f..ca95b7c780f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -55,6 +55,63 @@ Major New Features Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +- ``-Wextra-semi-stmt`` is a new diagnostic that diagnoses extra semicolons, + much like ``-Wextra-semi``. This new diagnostic diagnoses all *unnecessary* + null statements (expression statements without an expression), unless: the + semicolon directly follows a macro that was expanded to nothing or if the + semicolon is within the macro itself. This applies to macros defined in system + headers as well as user-defined macros. + + .. code-block:: c++ + + #define MACRO(x) int x; + #define NULLMACRO(varname) + + void test() { + ; // <- warning: ';' with no preceding expression is a null statement + + while (true) + ; // OK, it is needed. + + switch (my_enum) { + case E1: + // stuff + break; + case E2: + ; // OK, it is needed. + } + + MACRO(v0;) // Extra semicolon, but within macro, so ignored. + + MACRO(v1); // <- warning: ';' with no preceding expression is a null statement + + NULLMACRO(v2); // ignored, NULLMACRO expanded to nothing. + } + +- ``-Wempty-init-stmt`` is a new diagnostic that diagnoses empty init-statements + of ``if``, ``switch``, ``range-based for``, unless: the semicolon directly + follows a macro that was expanded to nothing or if the semicolon is within the + macro itself (both macros from system headers, and normal macros). This + diagnostic is in the ``-Wextra-semi-stmt`` group and is enabled in + ``-Wextra``. + + .. code-block:: c++ + + void test() { + if(; // <- warning: init-statement of 'if' is a null statement + true) + ; + + switch (; // <- warning: init-statement of 'switch' is a null statement + x) { + ... + } + + for (; // <- warning: init-statement of 'range-based for' is a null statement + int y : S()) + ; + } + Non-comprehensive list of changes in this release ------------------------------------------------- -- cgit v1.2.3