[cfe-dev] r347339 - [clang][Parse] Diagnose useless null statements / empty init-statements
James Y Knight via cfe-dev
cfe-dev at lists.llvm.org
Wed Dec 5 13:03:31 PST 2018
Nobody should be using "-Weverything" in their default build flags! It
should only be used as a way to find the names of interesting warning
flags. I'd note that "-Weverything" even has warnings that _conflict_ with
each-other...
It's unworkable for clang to have a policy which prevents the addition of
new warning options just because someone enables -Weverything... If people
feel like that is what's happening now, we need to fix that. If that's what
it comes to, even deleting the -Weverything flag would be better.
The question I'd ask for new warnings is a different one: is this warning
something which people could feasibly decide to *opt into*, with -Werror,
for their entire project? This does appear to meet that criteria -- the
cases where it warns do appear to be indicating potential issues, and the
code fix which silences the warning will improve the code, not make it
worse.
On Wed, Dec 5, 2018 at 2:51 PM Aaron Ballman via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> On Wed, Dec 5, 2018 at 2:14 PM Alex L via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> >
> > We have about 100k unique occurrences of this warning across a slice of
> our software stack. It's simply not feasible to us to adopt it, so we would
> prefer for it to be reverted to avoid downstream divergence in Apple's
> clang. Granted, these occur in projects that use -Weverything but those
> projects are typically understanding of warnings that they can turn off
> when they see that they provide useful value in terms of warnings about
> unintended behavior that's sometimes caught. This warning doesn't seem to
> catch unintended bugs and it would be a heroic battle to adopt it for us in
> a way where projects are somewhat satisfied. It seems to be a better fit
> for a clang-tidy check that could suggest automatic fixits and that the
> users could run on their codebase to "modernize" if they want to.
>
> I'm not certain I agree that this does not catch unintended bugs.
> After all, it started a discussion that pointed out design issues with
> two different macros which were provided as examples of
> false-positives. I think this diagnostic points out bad code smells
> and is no more stylistic than some of the other diagnostics in
> -Weverything, like not having a previous prototype before defining a
> function. I am not sold on the idea that this diagnostic should be
> moved into clang-tidy simply because it's showing up in a lot of
> -Weverything builds.
>
> ~Aaron
>
> >
> > Cheers,
> > Alex
> >
> > On Wed, 5 Dec 2018 at 10:43, Richard Smith via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> >>
> >> On Wed, 5 Dec 2018, 10:01 George Karpenkov via cfe-commits <
> cfe-commits at lists.llvm.org wrote:
> >>>
> >>> 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.
> >>
> >>
> >> 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).
> >>>
> >>> 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>
> 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
> >>>
> >>>
> >>> _______________________________________________
> >>> 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
> _______________________________________________
> 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-dev/attachments/20181205/84d6261f/attachment.html>
More information about the cfe-dev
mailing list