On Wed, Dec 5, 2018 at 1:40 AM Roman Lebedev <lebedev...@gmail.com> wrote: > > On Wed, Dec 5, 2018 at 4:07 AM Artem Dergachev via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > > > > > > > On 12/4/18 5:04 PM, George Karpenkov via cfe-dev wrote: > > > > > >> On Dec 4, 2018, at 4:47 PM, Aaron Ballman <aa...@aaronballman.com> wrote: > > >> > > >> On Tue, Dec 4, 2018 at 7:35 PM George Karpenkov <ekarpen...@apple.com> > > >> wrote: > > >>> Hi Roman, > Hi. > > > >>> I’m against this new warning. > > >>> > > >>> A very common idiom is defining a “DEBUG” macro to be a no-op in > > >>> release, and a logging function otherwise. > > >>> The new introduced warning warns on all such cases (e.g. > > >>> 'DEBUG(“mystring”);') > > >> Yeah, that's not good. > It does warn on such cases when the macro not expanded to nothing, yes. > > > >>> As noted in the review, Clang warnings should generally not be used to > > >>> diagnose style issues — and an extra semicolon does not seem to be a > > >>> common bug source. > I don't disagree. > The alternative solution would be to try to squeeze this bit of info > into the AST, > without increasing the size of the particular type, and then emit the > diag in clang-tidy. > I did consider it, it seemed like a over-engineering. > > > >>> 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? > > > >> 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@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@lists.llvm.org > > >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > _______________________________________________ > > > cfe-dev mailing list > > > cfe-...@lists.llvm.org > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev > > > > _______________________________________________ > > cfe-commits mailing list > > cfe-commits@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
