[cfe-dev] r347339 - [clang][Parse] Diagnose useless null statements / empty init-statements
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 5 10:00:44 PST 2018
These are the cases I get:
#define unexpected(a) { kprintf("unexpected %s:%d\n", __FILE__, __LINE__); a; }
and
#if 0
#define DEBG(fmt, args...) { IOLog(fmt, ## args); kprintf(fmt, ## args); }
#else
#define DEBG(fmt, args...) {}
#endif
Now calls
`DEBG(‘x’);` and `unexpected(‘msg’);`
cause the warning to be fired.
Of course, semicolon could be omitted, but the fact that the macro has braces is sort of an implementation detail.
Greg Parker has pointed out it’s possible to rewrite the macros using the “do { … } while (0)”,
but that is a lot of macros to change, and the resulting code would be arguably less readable.
> On Dec 5, 2018, at 5:10 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>
> On Wed, Dec 5, 2018 at 1:40 AM Roman Lebedev <lebedev.ri at gmail.com <mailto:lebedev.ri at gmail.com>> wrote:
>>
>>>>>> It is a problem in practice for us since we have projects which enable all available warnings and also enable “-Werror”.
>> Hm, they have asked for -Weverything and they got it. Seems to be
>> working as intended.
>> When in previous discussions elsewhere i have talked about
>> -Weverything, i was always
>> been told that it isn't supposed to be used like that (admittedly, i
>> *do* use it like that :)).
>>
>> Could you explain how is this different from any other warning that is
>> considered pointless by the project?
>> Why not simply disable it?
You are right, it could be disabled. It’s just for this warning the noise vs. usefulness ratio seemed pretty low.
>>
>>>>> Would you be okay with the warning if it was only diagnosed when the
>>>>> source location of the semi-colon is not immediately preceded by a
>>>>> macro expansion location? e.g.,
>>>>>
>>>>> EMPTY_EXPANSION(12); // Not diagnosed
>>>>> EMPTY_EXPANSION; // Not diagnosed
>>>>> ; // Diagnosed
>>>> 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).
>>>> I’m not sure the complexity would be worth it, as I haven’t seen bugs arising from extra semicolons and empty statements,
>>>> but it would take care of my use case.
>>>>
>>>
>>> Or rather when the *previous* semicolon is coming from a macro.
>> I don't like this. clang is already wa-a-ay too forgiving about
>> macros, it almost ignores almost every warning in macros.
>> It was an *intentional* decision to still warn on useless ';' after
>> non-empty macros.
>
> I think I'm confused now. This is a test case:
>
> NULLMACRO(v7); // OK. This could be either GOODMACRO() or
> BETTERMACRO() situation, so we can't know we can drop it.
>
> So empty macro expansions should be ignored as I expected... so what
> is being diagnosed? George, can you provide a more clear example that
> demonstrates the issue?
>
> To be clear, I do *not* think this is a situation we should spend
> effort supporting (assuming "all available warnings" really means
> -Weverything and not something else):
>
>> It is a problem in practice for us since we have projects which enable all available warnings and also enable “-Werror”.
>
> However, if we are diagnosing *empty* macro expansions followed by a
> semi-colon, I think that would be unacceptably chatty and is worth
> fixing on its own merits.
>
> ~Aaron
>
>>
>>>>> ~Aaron
>>>>>
>>>>>> Regards,
>>>>>> George
>> Roman.
>>
>>>>>>> On Nov 20, 2018, at 10:59 AM, Roman Lebedev via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>>>>>>>
>>>>>>> Author: lebedevri
>>>>>>> Date: Tue Nov 20 10:59:05 2018
>>>>>>> New Revision: 347339
>>>>>>>
>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=347339&view=rev
>>>>>>> Log:
>>>>>>> [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
>>>>>>>
>>>>>>> Added:
>>>>>>> cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
>>>>>>> cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp
>>>>>>> Modified:
>>>>>>> cfe/trunk/docs/ReleaseNotes.rst
>>>>>>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>>>>>>> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>>>>>>> cfe/trunk/include/clang/Parse/Parser.h
>>>>>>> cfe/trunk/lib/Parse/ParseExprCXX.cpp
>>>>>>> cfe/trunk/lib/Parse/ParseStmt.cpp
>>>>>>>
>>>>>>> Modified: cfe/trunk/docs/ReleaseNotes.rst
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=347339&r1=347338&r2=347339&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/docs/ReleaseNotes.rst (original)
>>>>>>> +++ cfe/trunk/docs/ReleaseNotes.rst Tue Nov 20 10:59:05 2018
>>>>>>> @@ -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
>>>>>>> -------------------------------------------------
>>>>>>>
>>>>>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=347339&r1=347338&r2=347339&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>>>>>>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Nov 20 10:59:05 2018
>>>>>>> @@ -162,6 +162,8 @@ def GNUEmptyStruct : DiagGroup<"gnu-empt
>>>>>>> def ExtraTokens : DiagGroup<"extra-tokens">;
>>>>>>> def CXX98CompatExtraSemi : DiagGroup<"c++98-compat-extra-semi">;
>>>>>>> def CXX11ExtraSemi : DiagGroup<"c++11-extra-semi">;
>>>>>>> +def EmptyInitStatement : DiagGroup<"empty-init-stmt">;
>>>>>>> +def ExtraSemiStmt : DiagGroup<"extra-semi-stmt", [EmptyInitStatement]>;
>>>>>>> def ExtraSemi : DiagGroup<"extra-semi", [CXX98CompatExtraSemi,
>>>>>>> CXX11ExtraSemi]>;
>>>>>>>
>>>>>>> @@ -768,7 +770,8 @@ def Extra : DiagGroup<"extra", [
>>>>>>> MissingMethodReturnType,
>>>>>>> SignCompare,
>>>>>>> UnusedParameter,
>>>>>>> - NullPointerArithmetic
>>>>>>> + NullPointerArithmetic,
>>>>>>> + EmptyInitStatement
>>>>>>> ]>;
>>>>>>>
>>>>>>> def Most : DiagGroup<"most", [
>>>>>>>
>>>>>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=347339&r1=347338&r2=347339&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
>>>>>>> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Tue Nov 20 10:59:05 2018
>>>>>>> @@ -53,6 +53,10 @@ def ext_extra_semi_cxx11 : Extension<
>>>>>>> def warn_extra_semi_after_mem_fn_def : Warning<
>>>>>>> "extra ';' after member function definition">,
>>>>>>> InGroup<ExtraSemi>, DefaultIgnore;
>>>>>>> +def warn_null_statement : Warning<
>>>>>>> + "empty expression statement has no effect; "
>>>>>>> + "remove unnecessary ';' to silence this warning">,
>>>>>>> + InGroup<ExtraSemiStmt>, DefaultIgnore;
>>>>>>>
>>>>>>> def ext_thread_before : Extension<"'__thread' before '%0'">;
>>>>>>> def ext_keyword_as_ident : ExtWarn<
>>>>>>> @@ -554,6 +558,9 @@ def ext_for_range_init_stmt : ExtWarn<
>>>>>>> def warn_cxx17_compat_for_range_init_stmt : Warning<
>>>>>>> "range-based for loop initialization statements are incompatible with "
>>>>>>> "C++ standards before C++2a">, DefaultIgnore, InGroup<CXXPre2aCompat>;
>>>>>>> +def warn_empty_init_statement : Warning<
>>>>>>> + "empty initialization statement of '%select{if|switch|range-based for}0' "
>>>>>>> + "has no effect">, InGroup<EmptyInitStatement>, DefaultIgnore;
>>>>>>>
>>>>>>> // C++ derived classes
>>>>>>> def err_dup_virtual : Error<"duplicate 'virtual' in base specifier">;
>>>>>>>
>>>>>>> Modified: cfe/trunk/include/clang/Parse/Parser.h
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=347339&r1=347338&r2=347339&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/include/clang/Parse/Parser.h (original)
>>>>>>> +++ cfe/trunk/include/clang/Parse/Parser.h Tue Nov 20 10:59:05 2018
>>>>>>> @@ -1888,6 +1888,7 @@ private:
>>>>>>> StmtResult ParseCompoundStatement(bool isStmtExpr,
>>>>>>> unsigned ScopeFlags);
>>>>>>> void ParseCompoundStatementLeadingPragmas();
>>>>>>> + bool ConsumeNullStmt(StmtVector &Stmts);
>>>>>>> StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
>>>>>>> bool ParseParenExprOrCondition(StmtResult *InitStmt,
>>>>>>> Sema::ConditionResult &CondResult,
>>>>>>>
>>>>>>> Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=347339&r1=347338&r2=347339&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
>>>>>>> +++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Tue Nov 20 10:59:05 2018
>>>>>>> @@ -1773,7 +1773,13 @@ Sema::ConditionResult Parser::ParseCXXCo
>>>>>>> // if (; true);
>>>>>>> if (InitStmt && Tok.is(tok::semi)) {
>>>>>>> WarnOnInit();
>>>>>>> - SourceLocation SemiLoc = ConsumeToken();
>>>>>>> + SourceLocation SemiLoc = Tok.getLocation();
>>>>>>> + if (!Tok.hasLeadingEmptyMacro() && !SemiLoc.isMacroID()) {
>>>>>>> + Diag(SemiLoc, diag::warn_empty_init_statement)
>>>>>>> + << (CK == Sema::ConditionKind::Switch)
>>>>>>> + << FixItHint::CreateRemoval(SemiLoc);
>>>>>>> + }
>>>>>>> + ConsumeToken();
>>>>>>> *InitStmt = Actions.ActOnNullStmt(SemiLoc);
>>>>>>> return ParseCXXCondition(nullptr, Loc, CK);
>>>>>>> }
>>>>>>>
>>>>>>> Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=347339&r1=347338&r2=347339&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
>>>>>>> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Tue Nov 20 10:59:05 2018
>>>>>>> @@ -930,6 +930,34 @@ void Parser::ParseCompoundStatementLeadi
>>>>>>>
>>>>>>> }
>>>>>>>
>>>>>>> +/// Consume any extra semi-colons resulting in null statements,
>>>>>>> +/// returning true if any tok::semi were consumed.
>>>>>>> +bool Parser::ConsumeNullStmt(StmtVector &Stmts) {
>>>>>>> + if (!Tok.is(tok::semi))
>>>>>>> + return false;
>>>>>>> +
>>>>>>> + SourceLocation StartLoc = Tok.getLocation();
>>>>>>> + SourceLocation EndLoc;
>>>>>>> +
>>>>>>> + while (Tok.is(tok::semi) && !Tok.hasLeadingEmptyMacro() &&
>>>>>>> + Tok.getLocation().isValid() && !Tok.getLocation().isMacroID()) {
>>>>>>> + EndLoc = Tok.getLocation();
>>>>>>> +
>>>>>>> + // Don't just ConsumeToken() this tok::semi, do store it in AST.
>>>>>>> + StmtResult R = ParseStatementOrDeclaration(Stmts, ACK_Any);
>>>>>>> + if (R.isUsable())
>>>>>>> + Stmts.push_back(R.get());
>>>>>>> + }
>>>>>>> +
>>>>>>> + // Did not consume any extra semi.
>>>>>>> + if (EndLoc.isInvalid())
>>>>>>> + return false;
>>>>>>> +
>>>>>>> + Diag(StartLoc, diag::warn_null_statement)
>>>>>>> + << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
>>>>>>> + return true;
>>>>>>> +}
>>>>>>> +
>>>>>>> /// ParseCompoundStatementBody - Parse a sequence of statements and invoke the
>>>>>>> /// ActOnCompoundStmt action. This expects the '{' to be the current token, and
>>>>>>> /// consume the '}' at the end of the block. It does not manipulate the scope
>>>>>>> @@ -992,6 +1020,9 @@ StmtResult Parser::ParseCompoundStatemen
>>>>>>> continue;
>>>>>>> }
>>>>>>>
>>>>>>> + if (ConsumeNullStmt(Stmts))
>>>>>>> + continue;
>>>>>>> +
>>>>>>> StmtResult R;
>>>>>>> if (Tok.isNot(tok::kw___extension__)) {
>>>>>>> R = ParseStatementOrDeclaration(Stmts, ACK_Any);
>>>>>>> @@ -1588,10 +1619,15 @@ StmtResult Parser::ParseForStatement(Sou
>>>>>>> ParsedAttributesWithRange attrs(AttrFactory);
>>>>>>> MaybeParseCXX11Attributes(attrs);
>>>>>>>
>>>>>>> + SourceLocation EmptyInitStmtSemiLoc;
>>>>>>> +
>>>>>>> // Parse the first part of the for specifier.
>>>>>>> if (Tok.is(tok::semi)) { // for (;
>>>>>>> ProhibitAttributes(attrs);
>>>>>>> // no first part, eat the ';'.
>>>>>>> + SourceLocation SemiLoc = Tok.getLocation();
>>>>>>> + if (!Tok.hasLeadingEmptyMacro() && !SemiLoc.isMacroID())
>>>>>>> + EmptyInitStmtSemiLoc = SemiLoc;
>>>>>>> ConsumeToken();
>>>>>>> } else if (getLangOpts().CPlusPlus && Tok.is(tok::identifier) &&
>>>>>>> isForRangeIdentifier()) {
>>>>>>> @@ -1723,6 +1759,11 @@ StmtResult Parser::ParseForStatement(Sou
>>>>>>> : diag::ext_for_range_init_stmt)
>>>>>>> << (FirstPart.get() ? FirstPart.get()->getSourceRange()
>>>>>>> : SourceRange());
>>>>>>> + if (EmptyInitStmtSemiLoc.isValid()) {
>>>>>>> + Diag(EmptyInitStmtSemiLoc, diag::warn_empty_init_statement)
>>>>>>> + << /*for-loop*/ 2
>>>>>>> + << FixItHint::CreateRemoval(EmptyInitStmtSemiLoc);
>>>>>>> + }
>>>>>>> }
>>>>>>> } else {
>>>>>>> ExprResult SecondExpr = ParseExpression();
>>>>>>>
>>>>>>> Added: cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp?rev=347339&view=auto
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp (added)
>>>>>>> +++ cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp Tue Nov 20 10:59:05 2018
>>>>>>> @@ -0,0 +1,64 @@
>>>>>>> +// RUN: %clang_cc1 -fsyntax-only -Wextra -std=c++2a -verify %s
>>>>>>> +// RUN: %clang_cc1 -fsyntax-only -Wextra-semi-stmt -std=c++2a -verify %s
>>>>>>> +// RUN: %clang_cc1 -fsyntax-only -Wempty-init-stmt -std=c++2a -verify %s
>>>>>>> +// RUN: cp %s %t
>>>>>>> +// RUN: %clang_cc1 -x c++ -Wempty-init-stmt -std=c++2a -fixit %t
>>>>>>> +// RUN: %clang_cc1 -x c++ -Wempty-init-stmt -std=c++2a -Werror %t
>>>>>>> +
>>>>>>> +struct S {
>>>>>>> + int *begin();
>>>>>>> + int *end();
>>>>>>> +};
>>>>>>> +
>>>>>>> +void naive(int x) {
>>>>>>> + if (; true) // expected-warning {{empty initialization statement of 'if' has no effect}}
>>>>>>> + ;
>>>>>>> +
>>>>>>> + switch (; x) { // expected-warning {{empty initialization statement of 'switch' has no effect}}
>>>>>>> + }
>>>>>>> +
>>>>>>> + for (; int y : S()) // expected-warning {{empty initialization statement of 'range-based for' has no effect}}
>>>>>>> + ;
>>>>>>> +
>>>>>>> + for (;;) // OK
>>>>>>> + ;
>>>>>>> +}
>>>>>>> +
>>>>>>> +#define NULLMACRO
>>>>>>> +
>>>>>>> +void with_null_macro(int x) {
>>>>>>> + if (NULLMACRO; true)
>>>>>>> + ;
>>>>>>> +
>>>>>>> + switch (NULLMACRO; x) {
>>>>>>> + }
>>>>>>> +
>>>>>>> + for (NULLMACRO; int y : S())
>>>>>>> + ;
>>>>>>> +}
>>>>>>> +
>>>>>>> +#define SEMIMACRO ;
>>>>>>> +
>>>>>>> +void with_semi_macro(int x) {
>>>>>>> + if (SEMIMACRO true)
>>>>>>> + ;
>>>>>>> +
>>>>>>> + switch (SEMIMACRO x) {
>>>>>>> + }
>>>>>>> +
>>>>>>> + for (SEMIMACRO int y : S())
>>>>>>> + ;
>>>>>>> +}
>>>>>>> +
>>>>>>> +#define PASSTHROUGHMACRO(x) x
>>>>>>> +
>>>>>>> +void with_passthrough_macro(int x) {
>>>>>>> + if (PASSTHROUGHMACRO(;) true)
>>>>>>> + ;
>>>>>>> +
>>>>>>> + switch (PASSTHROUGHMACRO(;) x) {
>>>>>>> + }
>>>>>>> +
>>>>>>> + for (PASSTHROUGHMACRO(;) int y : S())
>>>>>>> + ;
>>>>>>> +}
>>>>>>>
>>>>>>> Added: cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp?rev=347339&view=auto
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp (added)
>>>>>>> +++ cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp Tue Nov 20 10:59:05 2018
>>>>>>> @@ -0,0 +1,96 @@
>>>>>>> +// RUN: %clang_cc1 -fsyntax-only -Wextra-semi-stmt -verify %s
>>>>>>> +// RUN: cp %s %t
>>>>>>> +// RUN: %clang_cc1 -x c++ -Wextra-semi-stmt -fixit %t
>>>>>>> +// RUN: %clang_cc1 -x c++ -Wextra-semi-stmt -Werror %t
>>>>>>> +
>>>>>>> +#define GOODMACRO(varname) int varname
>>>>>>> +#define BETTERMACRO(varname) GOODMACRO(varname);
>>>>>>> +#define NULLMACRO(varname)
>>>>>>> +
>>>>>>> +enum MyEnum {
>>>>>>> + E1,
>>>>>>> + E2
>>>>>>> +};
>>>>>>> +
>>>>>>> +void test() {
>>>>>>> + ; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
>>>>>>> + ;
>>>>>>> +
>>>>>>> + // This removal of extra semi also consumes all the comments.
>>>>>>> + // clang-format: off
>>>>>>> + ;;;
>>>>>>> + // clang-format: on
>>>>>>> +
>>>>>>> + // clang-format: off
>>>>>>> + ;NULLMACRO(ZZ);
>>>>>>> + // clang-format: on
>>>>>>> +
>>>>>>> + {}; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
>>>>>>> +
>>>>>>> + {
>>>>>>> + ; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (true) {
>>>>>>> + ; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
>>>>>>> + }
>>>>>>> +
>>>>>>> + GOODMACRO(v0); // OK
>>>>>>> +
>>>>>>> + GOODMACRO(v1;) // OK
>>>>>>> +
>>>>>>> + BETTERMACRO(v2) // OK
>>>>>>> +
>>>>>>> + BETTERMACRO(v3;) // Extra ';', but within macro expansion, so ignored.
>>>>>>> +
>>>>>>> + BETTERMACRO(v4); // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
>>>>>>> +
>>>>>>> + BETTERMACRO(v5;); // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
>>>>>>> +
>>>>>>> + NULLMACRO(v6) // OK
>>>>>>> +
>>>>>>> + NULLMACRO(v7); // OK. This could be either GOODMACRO() or BETTERMACRO() situation, so we can't know we can drop it.
>>>>>>> +
>>>>>>> + if (true)
>>>>>>> + ; // OK
>>>>>>> +
>>>>>>> + while (true)
>>>>>>> + ; // OK
>>>>>>> +
>>>>>>> + do
>>>>>>> + ; // OK
>>>>>>> + while (true);
>>>>>>> +
>>>>>>> + for (;;) // OK
>>>>>>> + ; // OK
>>>>>>> +
>>>>>>> + MyEnum my_enum;
>>>>>>> + switch (my_enum) {
>>>>>>> + case E1:
>>>>>>> + // stuff
>>>>>>> + break;
>>>>>>> + case E2:; // OK
>>>>>>> + }
>>>>>>> +
>>>>>>> + for (;;) {
>>>>>>> + for (;;) {
>>>>>>> + goto contin_outer;
>>>>>>> + }
>>>>>>> + contin_outer:; // OK
>>>>>>> + }
>>>>>>> +}
>>>>>>> +
>>>>>>> +;
>>>>>>> +
>>>>>>> +namespace NS {};
>>>>>>> +
>>>>>>> +void foo(int x) {
>>>>>>> + switch (x) {
>>>>>>> + case 0:
>>>>>>> + [[fallthrough]];
>>>>>>> + case 1:
>>>>>>> + return;
>>>>>>> + }
>>>>>>> +}
>>>>>>> +
>>>>>>> +[[]];
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> cfe-commits mailing list
>>>>>>> cfe-commits at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>> _______________________________________________
>>>> cfe-dev mailing list
>>>> cfe-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181205/4e144292/attachment-0001.html>
More information about the cfe-commits
mailing list