[cfe-dev] r347339 - [clang][Parse] Diagnose useless null statements / empty init-statements
Aaron Ballman via cfe-dev
cfe-dev at lists.llvm.org
Wed Dec 5 05:10:02 PST 2018
On Wed, Dec 5, 2018 at 1:40 AM Roman Lebedev <lebedev.ri at gmail.com> wrote:
>
> On Wed, Dec 5, 2018 at 4:07 AM Artem Dergachev via cfe-commits
> <cfe-commits at 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 <aaron at aaronballman.com> wrote:
> > >>
> > >> On Tue, Dec 4, 2018 at 7:35 PM George Karpenkov <ekarpenkov at 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 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
More information about the cfe-dev
mailing list