<div dir="auto"><div><div class="gmail_quote"><div dir="ltr">On Wed, 5 Dec 2018, 10:01 George Karpenkov via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">These are the cases I get:<div><br></div><div>#define unexpected(a) { kprintf("unexpected %s:%d\n", __FILE__, __LINE__); a; }</div><div><br></div><div>and</div><div><br></div><div><div>#if 0</div><div>#define DEBG(fmt, args...) { IOLog(fmt, ## args); kprintf(fmt, ## args); }</div><div>#else</div><div>#define DEBG(fmt, args...) {}</div><div>#endif</div><div><br></div><div>Now calls</div><div><br></div><div>`DEBG(‘x’);` and `unexpected(‘msg’);`</div><div><br></div><div>cause the warning to be fired.</div><div>Of course, semicolon could be omitted, but the fact that the macro has braces is sort of an implementation detail.</div><div><br></div><div>Greg Parker has pointed out it’s possible to rewrite the macros using the “do { … } while (0)”,</div><div>but that is a lot of macros to change, and the resulting code would be arguably less readable.</div></div></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">That suggestion is the (or at least an) idiomatic way to write a "statement-like" macro that expects a semicolon. The approach taken by the above macros is often considered to be a macro antipattern (try putting it in an unbraced if and adding an else clause, for example).</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><div><blockquote type="cite"><div>On Dec 5, 2018, at 5:10 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank" rel="noreferrer">aaron@aaronballman.com</a>> wrote:</div><br class="m_-4464857630312646578Apple-interchange-newline"><div><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">On Wed, Dec 5, 2018 at 1:40 AM Roman Lebedev <</span><a href="mailto:lebedev.ri@gmail.com" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" target="_blank" rel="noreferrer">lebedev.ri@gmail.com</a><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">> wrote:</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">It is a problem in practice for us since we have projects which enable all available warnings and also enable “-Werror”.<br></blockquote></blockquote></blockquote></blockquote>Hm, they have asked for -Weverything and they got it. Seems to be<br>working as intended.<br>When in previous discussions elsewhere i have talked about<br>-Weverything, i was always<br>been told that it isn't supposed to be used like that (admittedly, i<br>*do* use it like that :)).<br><br>Could you explain how is this different from any other warning that is<br>considered pointless by the project?<br>Why not simply disable it?<br></blockquote></div></blockquote><div><br></div><div>You are right, it could be disabled. It’s just for this warning the noise vs. usefulness ratio seemed pretty low.</div><br><blockquote type="cite"><div><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">Would you be okay with the warning if it was only diagnosed when the<br>source location of the semi-colon is not immediately preceded by a<br>macro expansion location? e.g.,<br><br>EMPTY_EXPANSION(12); // Not diagnosed<br>EMPTY_EXPANSION; // Not diagnosed<br>; // Diagnosed<br></blockquote>This could work provided that all empty space preceding the semicolon is ignored (as the macro could be separated by space(s) or newline(s).<br>I’m not sure the complexity would be worth it, as I haven’t seen bugs arising from extra semicolons and empty statements,<br>but it would take care of my use case.<br><br></blockquote><br>Or rather when the *previous* semicolon is coming from a macro.<br></blockquote>I don't like this. clang is already wa-a-ay too forgiving about<br>macros, it almost ignores almost every warning in macros.<br>It was an *intentional* decision to still warn on useless ';' after<br>non-empty macros.<br></blockquote><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">I think I'm confused now. This is a test case:</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">NULLMACRO(v7); // OK. This could be either GOODMACRO() or</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">BETTERMACRO() situation, so we can't know we can drop it.</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">So empty macro expansions should be ignored as I expected... so what</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">is being diagnosed? George, can you provide a more clear example that</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">demonstrates the issue?</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">To be clear, I do *not* think this is a situation we should spend</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">effort supporting (assuming "all available warnings" really means</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">-Weverything and not something else):</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none">It is a problem in practice for us since we have projects which enable all available warnings and also enable “-Werror”.<br></blockquote><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">However, if we are diagnosing *empty* macro expansions followed by a</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">semi-colon, I think that would be unacceptably chatty and is worth</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">fixing on its own merits.</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important">~Aaron</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">~Aaron<br><br><blockquote type="cite">Regards,<br>George<br></blockquote></blockquote></blockquote></blockquote>Roman.<br><br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">On Nov 20, 2018, at 10:59 AM, Roman Lebedev via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" rel="noreferrer">cfe-commits@lists.llvm.org</a>> wrote:<br><br>Author: lebedevri<br>Date: Tue Nov 20 10:59:05 2018<br>New Revision: 347339<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=347339&view=rev" target="_blank" rel="noreferrer">http://llvm.org/viewvc/llvm-project?rev=347339&view=rev</a><br>Log:<br>[clang][Parse] Diagnose useless null statements / empty init-statements<br><br>Summary:<br>clang has `-Wextra-semi` (D43162), which is not dictated by the currently selected standard.<br>While that is great, there is at least one more source of need-less semis - 'null statements'.<br>Sometimes, they are needed:<br>```<br>for(int x = 0; continueToDoWork(x); x++)<br>; // Ugly code, but the semi is needed here.<br>```<br><br>But sometimes they are just there for no reason:<br>```<br>switch(X) {<br>case 0:<br>return -2345;<br>case 5:<br>return 0;<br>default:<br>return 42;<br>}; // <- oops<br><br>;;;;;;;;;;; <- OOOOPS, still not diagnosed. Clearly this is junk.<br>```<br><br>Additionally:<br>```<br>if(; // <- empty init-statement<br> true)<br>;<br><br>switch (; // empty init-statement<br> x) {<br>...<br>}<br><br>for (; // <- empty init-statement<br> int y : S())<br>;<br>}<br><br>As usual, things may or may not go sideways in the presence of macros.<br>While evaluating this diag on my codebase of interest, it was unsurprisingly<br>discovered that Google Test macros are *very* prone to this.<br>And it seems many issues are deep within the GTest itself, not<br>in the snippets passed from the codebase that uses GTest.<br><br>So after some thought, i decided not do issue a diagnostic if the semi<br>is within *any* macro, be it either from the normal header, or system header.<br><br>Fixes [[ <a href="https://bugs.llvm.org/show_bug.cgi?id=39111" target="_blank" rel="noreferrer">https://bugs.llvm.org/show_bug.cgi?id=39111</a> | PR39111 ]]<br><br>Reviewers: rsmith, aaron.ballman, efriedma<br><br>Reviewed By: aaron.ballman<br><br>Subscribers: cfe-commits<br><br>Differential Revision: <a href="https://reviews.llvm.org/D52695" target="_blank" rel="noreferrer">https://reviews.llvm.org/D52695</a><br><br>Added:<br> cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp<br> cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp<br>Modified:<br> cfe/trunk/docs/ReleaseNotes.rst<br> cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<br> cfe/trunk/include/clang/Parse/Parser.h<br> cfe/trunk/lib/Parse/ParseExprCXX.cpp<br> cfe/trunk/lib/Parse/ParseStmt.cpp<br><br>Modified: cfe/trunk/docs/ReleaseNotes.rst<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=347339&r1=347338&r2=347339&view=diff" target="_blank" rel="noreferrer">http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=347339&r1=347338&r2=347339&view=diff</a><br>==============================================================================<br>--- cfe/trunk/docs/ReleaseNotes.rst (original)<br>+++ cfe/trunk/docs/ReleaseNotes.rst Tue Nov 20 10:59:05 2018<br>@@ -55,6 +55,63 @@ Major New Features<br>Improvements to Clang's diagnostics<br>^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^<br><br>+- ``-Wextra-semi-stmt`` is a new diagnostic that diagnoses extra semicolons,<br>+ much like ``-Wextra-semi``. This new diagnostic diagnoses all *unnecessary*<br>+ null statements (expression statements without an expression), unless: the<br>+ semicolon directly follows a macro that was expanded to nothing or if the<br>+ semicolon is within the macro itself. This applies to macros defined in system<br>+ headers as well as user-defined macros.<br>+<br>+ .. code-block:: c++<br>+<br>+ #define MACRO(x) int x;<br>+ #define NULLMACRO(varname)<br>+<br>+ void test() {<br>+ ; // <- warning: ';' with no preceding expression is a null statement<br>+<br>+ while (true)<br>+ ; // OK, it is needed.<br>+<br>+ switch (my_enum) {<br>+ case E1:<br>+ // stuff<br>+ break;<br>+ case E2:<br>+ ; // OK, it is needed.<br>+ }<br>+<br>+ MACRO(v0;) // Extra semicolon, but within macro, so ignored.<br>+<br>+ MACRO(v1); // <- warning: ';' with no preceding expression is a null statement<br>+<br>+ NULLMACRO(v2); // ignored, NULLMACRO expanded to nothing.<br>+ }<br>+<br>+- ``-Wempty-init-stmt`` is a new diagnostic that diagnoses empty init-statements<br>+ of ``if``, ``switch``, ``range-based for``, unless: the semicolon directly<br>+ follows a macro that was expanded to nothing or if the semicolon is within the<br>+ macro itself (both macros from system headers, and normal macros). This<br>+ diagnostic is in the ``-Wextra-semi-stmt`` group and is enabled in<br>+ ``-Wextra``.<br>+<br>+ .. code-block:: c++<br>+<br>+ void test() {<br>+ if(; // <- warning: init-statement of 'if' is a null statement<br>+ true)<br>+ ;<br>+<br>+ switch (; // <- warning: init-statement of 'switch' is a null statement<br>+ x) {<br>+ ...<br>+ }<br>+<br>+ for (; // <- warning: init-statement of 'range-based for' is a null statement<br>+ int y : S())<br>+ ;<br>+ }<br>+<br><br>Non-comprehensive list of changes in this release<br>-------------------------------------------------<br><br>Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=347339&r1=347338&r2=347339&view=diff" target="_blank" rel="noreferrer">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=347339&r1=347338&r2=347339&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)<br>+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Nov 20 10:59:05 2018<br>@@ -162,6 +162,8 @@ def GNUEmptyStruct : DiagGroup<"gnu-empt<br>def ExtraTokens : DiagGroup<"extra-tokens">;<br>def CXX98CompatExtraSemi : DiagGroup<"c++98-compat-extra-semi">;<br>def CXX11ExtraSemi : DiagGroup<"c++11-extra-semi">;<br>+def EmptyInitStatement : DiagGroup<"empty-init-stmt">;<br>+def ExtraSemiStmt : DiagGroup<"extra-semi-stmt", [EmptyInitStatement]>;<br>def ExtraSemi : DiagGroup<"extra-semi", [CXX98CompatExtraSemi,<br> CXX11ExtraSemi]>;<br><br>@@ -768,7 +770,8 @@ def Extra : DiagGroup<"extra", [<br> MissingMethodReturnType,<br> SignCompare,<br> UnusedParameter,<br>- NullPointerArithmetic<br>+ NullPointerArithmetic,<br>+ EmptyInitStatement<br> ]>;<br><br>def Most : DiagGroup<"most", [<br><br>Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=347339&r1=347338&r2=347339&view=diff" target="_blank" rel="noreferrer">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=347339&r1=347338&r2=347339&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)<br>+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Tue Nov 20 10:59:05 2018<br>@@ -53,6 +53,10 @@ def ext_extra_semi_cxx11 : Extension<<br>def warn_extra_semi_after_mem_fn_def : Warning<<br> "extra ';' after member function definition">,<br> InGroup<ExtraSemi>, DefaultIgnore;<br>+def warn_null_statement : Warning<<br>+ "empty expression statement has no effect; "<br>+ "remove unnecessary ';' to silence this warning">,<br>+ InGroup<ExtraSemiStmt>, DefaultIgnore;<br><br>def ext_thread_before : Extension<"'__thread' before '%0'">;<br>def ext_keyword_as_ident : ExtWarn<<br>@@ -554,6 +558,9 @@ def ext_for_range_init_stmt : ExtWarn<<br>def warn_cxx17_compat_for_range_init_stmt : Warning<<br> "range-based for loop initialization statements are incompatible with "<br> "C++ standards before C++2a">, DefaultIgnore, InGroup<CXXPre2aCompat>;<br>+def warn_empty_init_statement : Warning<<br>+ "empty initialization statement of '%select{if|switch|range-based for}0' "<br>+ "has no effect">, InGroup<EmptyInitStatement>, DefaultIgnore;<br><br>// C++ derived classes<br>def err_dup_virtual : Error<"duplicate 'virtual' in base specifier">;<br><br>Modified: cfe/trunk/include/clang/Parse/Parser.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=347339&r1=347338&r2=347339&view=diff" target="_blank" rel="noreferrer">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=347339&r1=347338&r2=347339&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/Parse/Parser.h (original)<br>+++ cfe/trunk/include/clang/Parse/Parser.h Tue Nov 20 10:59:05 2018<br>@@ -1888,6 +1888,7 @@ private:<br> StmtResult ParseCompoundStatement(bool isStmtExpr,<br> unsigned ScopeFlags);<br> void ParseCompoundStatementLeadingPragmas();<br>+ bool ConsumeNullStmt(StmtVector &Stmts);<br> StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);<br> bool ParseParenExprOrCondition(StmtResult *InitStmt,<br> Sema::ConditionResult &CondResult,<br><br>Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=347339&r1=347338&r2=347339&view=diff" target="_blank" rel="noreferrer">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=347339&r1=347338&r2=347339&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)<br>+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Tue Nov 20 10:59:05 2018<br>@@ -1773,7 +1773,13 @@ Sema::ConditionResult Parser::ParseCXXCo<br> // if (; true);<br> if (InitStmt && <a href="http://Tok.is" target="_blank" rel="noreferrer">Tok.is</a>(tok::semi)) {<br> WarnOnInit();<br>- SourceLocation SemiLoc = ConsumeToken();<br>+ SourceLocation SemiLoc = Tok.getLocation();<br>+ if (!Tok.hasLeadingEmptyMacro() && !SemiLoc.isMacroID()) {<br>+ Diag(SemiLoc, diag::warn_empty_init_statement)<br>+ << (CK == Sema::ConditionKind::Switch)<br>+ << FixItHint::CreateRemoval(SemiLoc);<br>+ }<br>+ ConsumeToken();<br> *InitStmt = Actions.ActOnNullStmt(SemiLoc);<br> return ParseCXXCondition(nullptr, Loc, CK);<br> }<br><br>Modified: cfe/trunk/lib/Parse/ParseStmt.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=347339&r1=347338&r2=347339&view=diff" target="_blank" rel="noreferrer">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=347339&r1=347338&r2=347339&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)<br>+++ cfe/trunk/lib/Parse/ParseStmt.cpp Tue Nov 20 10:59:05 2018<br>@@ -930,6 +930,34 @@ void Parser::ParseCompoundStatementLeadi<br><br>}<br><br>+/// Consume any extra semi-colons resulting in null statements,<br>+/// returning true if any tok::semi were consumed.<br>+bool Parser::ConsumeNullStmt(StmtVector &Stmts) {<br>+ if (!<a href="http://Tok.is" target="_blank" rel="noreferrer">Tok.is</a>(tok::semi))<br>+ return false;<br>+<br>+ SourceLocation StartLoc = Tok.getLocation();<br>+ SourceLocation EndLoc;<br>+<br>+ while (<a href="http://Tok.is" target="_blank" rel="noreferrer">Tok.is</a>(tok::semi) && !Tok.hasLeadingEmptyMacro() &&<br>+ Tok.getLocation().isValid() && !Tok.getLocation().isMacroID()) {<br>+ EndLoc = Tok.getLocation();<br>+<br>+ // Don't just ConsumeToken() this tok::semi, do store it in AST.<br>+ StmtResult R = ParseStatementOrDeclaration(Stmts, ACK_Any);<br>+ if (R.isUsable())<br>+ Stmts.push_back(R.get());<br>+ }<br>+<br>+ // Did not consume any extra semi.<br>+ if (EndLoc.isInvalid())<br>+ return false;<br>+<br>+ Diag(StartLoc, diag::warn_null_statement)<br>+ << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));<br>+ return true;<br>+}<br>+<br>/// ParseCompoundStatementBody - Parse a sequence of statements and invoke the<br>/// ActOnCompoundStmt action. This expects the '{' to be the current token, and<br>/// consume the '}' at the end of the block. It does not manipulate the scope<br>@@ -992,6 +1020,9 @@ StmtResult Parser::ParseCompoundStatemen<br> continue;<br> }<br><br>+ if (ConsumeNullStmt(Stmts))<br>+ continue;<br>+<br> StmtResult R;<br> if (Tok.isNot(tok::kw___extension__)) {<br> R = ParseStatementOrDeclaration(Stmts, ACK_Any);<br>@@ -1588,10 +1619,15 @@ StmtResult Parser::ParseForStatement(Sou<br> ParsedAttributesWithRange attrs(AttrFactory);<br> MaybeParseCXX11Attributes(attrs);<br><br>+ SourceLocation EmptyInitStmtSemiLoc;<br>+<br> // Parse the first part of the for specifier.<br> if (<a href="http://Tok.is" target="_blank" rel="noreferrer">Tok.is</a>(tok::semi)) { // for (;<br> ProhibitAttributes(attrs);<br> // no first part, eat the ';'.<br>+ SourceLocation SemiLoc = Tok.getLocation();<br>+ if (!Tok.hasLeadingEmptyMacro() && !SemiLoc.isMacroID())<br>+ EmptyInitStmtSemiLoc = SemiLoc;<br> ConsumeToken();<br> } else if (getLangOpts().CPlusPlus && <a href="http://Tok.is" target="_blank" rel="noreferrer">Tok.is</a>(tok::identifier) &&<br> isForRangeIdentifier()) {<br>@@ -1723,6 +1759,11 @@ StmtResult Parser::ParseForStatement(Sou<br> : diag::ext_for_range_init_stmt)<br> << (FirstPart.get() ? FirstPart.get()->getSourceRange()<br> : SourceRange());<br>+ if (EmptyInitStmtSemiLoc.isValid()) {<br>+ Diag(EmptyInitStmtSemiLoc, diag::warn_empty_init_statement)<br>+ << /*for-loop*/ 2<br>+ << FixItHint::CreateRemoval(EmptyInitStmtSemiLoc);<br>+ }<br> }<br> } else {<br> ExprResult SecondExpr = ParseExpression();<br><br>Added: cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp?rev=347339&view=auto" target="_blank" rel="noreferrer">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp?rev=347339&view=auto</a><br>==============================================================================<br>--- cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp (added)<br>+++ cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp Tue Nov 20 10:59:05 2018<br>@@ -0,0 +1,64 @@<br>+// RUN: %clang_cc1 -fsyntax-only -Wextra -std=c++2a -verify %s<br>+// RUN: %clang_cc1 -fsyntax-only -Wextra-semi-stmt -std=c++2a -verify %s<br>+// RUN: %clang_cc1 -fsyntax-only -Wempty-init-stmt -std=c++2a -verify %s<br>+// RUN: cp %s %t<br>+// RUN: %clang_cc1 -x c++ -Wempty-init-stmt -std=c++2a -fixit %t<br>+// RUN: %clang_cc1 -x c++ -Wempty-init-stmt -std=c++2a -Werror %t<br>+<br>+struct S {<br>+ int *begin();<br>+ int *end();<br>+};<br>+<br>+void naive(int x) {<br>+ if (; true) // expected-warning {{empty initialization statement of 'if' has no effect}}<br>+ ;<br>+<br>+ switch (; x) { // expected-warning {{empty initialization statement of 'switch' has no effect}}<br>+ }<br>+<br>+ for (; int y : S()) // expected-warning {{empty initialization statement of 'range-based for' has no effect}}<br>+ ;<br>+<br>+ for (;;) // OK<br>+ ;<br>+}<br>+<br>+#define NULLMACRO<br>+<br>+void with_null_macro(int x) {<br>+ if (NULLMACRO; true)<br>+ ;<br>+<br>+ switch (NULLMACRO; x) {<br>+ }<br>+<br>+ for (NULLMACRO; int y : S())<br>+ ;<br>+}<br>+<br>+#define SEMIMACRO ;<br>+<br>+void with_semi_macro(int x) {<br>+ if (SEMIMACRO true)<br>+ ;<br>+<br>+ switch (SEMIMACRO x) {<br>+ }<br>+<br>+ for (SEMIMACRO int y : S())<br>+ ;<br>+}<br>+<br>+#define PASSTHROUGHMACRO(x) x<br>+<br>+void with_passthrough_macro(int x) {<br>+ if (PASSTHROUGHMACRO(;) true)<br>+ ;<br>+<br>+ switch (PASSTHROUGHMACRO(;) x) {<br>+ }<br>+<br>+ for (PASSTHROUGHMACRO(;) int y : S())<br>+ ;<br>+}<br><br>Added: cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp?rev=347339&view=auto" target="_blank" rel="noreferrer">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp?rev=347339&view=auto</a><br>==============================================================================<br>--- cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp (added)<br>+++ cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp Tue Nov 20 10:59:05 2018<br>@@ -0,0 +1,96 @@<br>+// RUN: %clang_cc1 -fsyntax-only -Wextra-semi-stmt -verify %s<br>+// RUN: cp %s %t<br>+// RUN: %clang_cc1 -x c++ -Wextra-semi-stmt -fixit %t<br>+// RUN: %clang_cc1 -x c++ -Wextra-semi-stmt -Werror %t<br>+<br>+#define GOODMACRO(varname) int varname<br>+#define BETTERMACRO(varname) GOODMACRO(varname);<br>+#define NULLMACRO(varname)<br>+<br>+enum MyEnum {<br>+ E1,<br>+ E2<br>+};<br>+<br>+void test() {<br>+ ; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}<br>+ ;<br>+<br>+ // This removal of extra semi also consumes all the comments.<br>+ // clang-format: off<br>+ ;;;<br>+ // clang-format: on<br>+<br>+ // clang-format: off<br>+ ;NULLMACRO(ZZ);<br>+ // clang-format: on<br>+<br>+ {}; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}<br>+<br>+ {<br>+ ; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}<br>+ }<br>+<br>+ if (true) {<br>+ ; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}<br>+ }<br>+<br>+ GOODMACRO(v0); // OK<br>+<br>+ GOODMACRO(v1;) // OK<br>+<br>+ BETTERMACRO(v2) // OK<br>+<br>+ BETTERMACRO(v3;) // Extra ';', but within macro expansion, so ignored.<br>+<br>+ BETTERMACRO(v4); // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}<br>+<br>+ BETTERMACRO(v5;); // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}<br>+<br>+ NULLMACRO(v6) // OK<br>+<br>+ NULLMACRO(v7); // OK. This could be either GOODMACRO() or BETTERMACRO() situation, so we can't know we can drop it.<br>+<br>+ if (true)<br>+ ; // OK<br>+<br>+ while (true)<br>+ ; // OK<br>+<br>+ do<br>+ ; // OK<br>+ while (true);<br>+<br>+ for (;;) // OK<br>+ ; // OK<br>+<br>+ MyEnum my_enum;<br>+ switch (my_enum) {<br>+ case E1:<br>+ // stuff<br>+ break;<br>+ case E2:; // OK<br>+ }<br>+<br>+ for (;;) {<br>+ for (;;) {<br>+ goto contin_outer;<br>+ }<br>+ contin_outer:; // OK<br>+ }<br>+}<br>+<br>+;<br>+<br>+namespace NS {};<br>+<br>+void foo(int x) {<br>+ switch (x) {<br>+ case 0:<br>+ [[fallthrough]];<br>+ case 1:<br>+ return;<br>+ }<br>+}<br>+<br>+[[]];<br><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@lists.llvm.org" target="_blank" rel="noreferrer">cfe-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank" rel="noreferrer">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br></blockquote></blockquote></blockquote>_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@lists.llvm.org" target="_blank" rel="noreferrer">cfe-dev@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank" rel="noreferrer">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br></blockquote><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@lists.llvm.org" target="_blank" rel="noreferrer">cfe-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank" rel="noreferrer">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a></blockquote></blockquote></div></blockquote></div><br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" rel="noreferrer">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div>