[llvm] r305506 - Added braces to work around gcc warning in googletest: suggest explicit braces to avoid ambiguous 'else'. NFC.

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 19 09:35:29 PDT 2017


Could we disable the GCC warning and keep Clang's that doesn't have this
false positive?

On Thu, Jun 15, 2017 at 2:14 PM Reid Kleckner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> GTest actually uses this crazy braceless switch/default pattern to
> suppress these warnings:
>
> // The GNU compiler emits a warning if nested "if" statements are followed
> by
> // an "else" statement and braces are not used to explicitly disambiguate
> the
> // "else" binding.  This leads to problems with code like:
> //
> //   if (gate)
> //     ASSERT_*(condition) << "Some message";
> //
> // The "switch (0) case 0:" idiom is used to suppress this.
> #ifdef __INTEL_COMPILER
> # define GTEST_AMBIGUOUS_ELSE_BLOCKER_
> #else
> # define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default:  //
> NOLINT
> #endif
>
> It sounds like you are compiling with a new version of GCC for which this
> idiom doesn't suppress the warning.
>
> Could you please file a bug with gtest with your gcc version to
> investigate and work around these warnings?
>
> On Thu, Jun 15, 2017 at 2:00 PM, Galina Kistanova via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: gkistanova
>> Date: Thu Jun 15 16:00:40 2017
>> New Revision: 305506
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=305506&view=rev
>> Log:
>> Added braces to work around gcc warning in googletest: suggest explicit
>> braces to avoid ambiguous 'else'. NFC.
>>
>> Modified:
>>     llvm/trunk/unittests/ADT/SCCIteratorTest.cpp
>>     llvm/trunk/unittests/ADT/StringRefTest.cpp
>>     llvm/trunk/unittests/CodeGen/LowLevelTypeTest.cpp
>>     llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
>>     llvm/trunk/unittests/Support/CommandLineTest.cpp
>>
>> Modified: llvm/trunk/unittests/ADT/SCCIteratorTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/SCCIteratorTest.cpp?rev=305506&r1=305505&r2=305506&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ADT/SCCIteratorTest.cpp (original)
>> +++ llvm/trunk/unittests/ADT/SCCIteratorTest.cpp Thu Jun 15 16:00:40 2017
>> @@ -63,8 +63,9 @@ TEST(SCCIteratorTest, AllSmallGraphs) {
>>        // Check that every node in the SCC is reachable from every other
>> node in
>>        // the SCC.
>>        for (unsigned i = 0; i != NUM_NODES; ++i)
>> -        if (NodesInThisSCC.count(i))
>> +        if (NodesInThisSCC.count(i)) {
>>
>>  EXPECT_TRUE(NodesInThisSCC.isSubsetOf(G.NodesReachableFrom(i)));
>> +        }
>>
>>        // OK, now that we now that every node in the SCC is reachable
>> from every
>>        // other, this means that the set of nodes reachable from any node
>> in the
>> @@ -78,8 +79,9 @@ TEST(SCCIteratorTest, AllSmallGraphs) {
>>              NodesReachableFromSCC.Meet(NodesInThisSCC.Complement());
>>
>>            for (unsigned j = 0; j != NUM_NODES; ++j)
>> -            if (ReachableButNotInSCC.count(j))
>> +            if (ReachableButNotInSCC.count(j)) {
>>
>>  EXPECT_TRUE(G.NodesReachableFrom(j).Meet(NodesInThisSCC).isEmpty());
>> +            }
>>
>>            // The result must be the same for all other nodes in this
>> SCC, so
>>            // there is no point in checking them.
>>
>> Modified: llvm/trunk/unittests/ADT/StringRefTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringRefTest.cpp?rev=305506&r1=305505&r2=305506&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ADT/StringRefTest.cpp (original)
>> +++ llvm/trunk/unittests/ADT/StringRefTest.cpp Thu Jun 15 16:00:40 2017
>> @@ -882,8 +882,9 @@ TEST(StringRefTest, getAsDouble) {
>>      double Result;
>>      StringRef S(Entry.Str);
>>      EXPECT_EQ(Entry.ShouldFail, S.getAsDouble(Result,
>> Entry.AllowInexact));
>> -    if (!Entry.ShouldFail)
>> +    if (!Entry.ShouldFail) {
>>        EXPECT_EQ(Result, Entry.D);
>> +    }
>>    }
>>  }
>>
>>
>> Modified: llvm/trunk/unittests/CodeGen/LowLevelTypeTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/CodeGen/LowLevelTypeTest.cpp?rev=305506&r1=305505&r2=305506&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/CodeGen/LowLevelTypeTest.cpp (original)
>> +++ llvm/trunk/unittests/CodeGen/LowLevelTypeTest.cpp Thu Jun 15 16:00:40
>> 2017
>> @@ -137,8 +137,9 @@ TEST(LowLevelTypeTest, Vector) {
>>        if ((Elts % 2) == 0) {
>>          EXPECT_EQ(S * (Elts / 2), HalfEltIfEvenTy.getSizeInBits());
>>          EXPECT_EQ(S, HalfEltIfEvenTy.getScalarSizeInBits());
>> -        if (Elts > 2)
>> +        if (Elts > 2) {
>>            EXPECT_EQ(Elts / 2, HalfEltIfEvenTy.getNumElements());
>> +        }
>>        }
>>
>>        EXPECT_EQ(S * (Elts * 2), DoubleEltTy.getSizeInBits());
>>
>> Modified: llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp?rev=305506&r1=305505&r2=305506&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp (original)
>> +++ llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp Thu Jun
>> 15 16:00:40 2017
>> @@ -311,8 +311,9 @@ void TestAllForms() {
>>    EXPECT_EQ(Data2, toReference(DieDG.find(Attr_DW_FORM_ref2), 0));
>>    EXPECT_EQ(Data4, toReference(DieDG.find(Attr_DW_FORM_ref4), 0));
>>    EXPECT_EQ(Data8, toReference(DieDG.find(Attr_DW_FORM_ref8), 0));
>> -  if (Version >= 4)
>> +  if (Version >= 4) {
>>      EXPECT_EQ(Data8_2, toReference(DieDG.find(Attr_DW_FORM_ref_sig8),
>> 0));
>> +  }
>>    EXPECT_EQ(UData[0], toReference(DieDG.find(Attr_DW_FORM_ref_udata),
>> 0));
>>
>>
>>  //----------------------------------------------------------------------
>> @@ -320,15 +321,17 @@ void TestAllForms() {
>>
>>  //----------------------------------------------------------------------
>>    EXPECT_EQ(1ULL, toUnsigned(DieDG.find(Attr_DW_FORM_flag_true), 0));
>>    EXPECT_EQ(0ULL, toUnsigned(DieDG.find(Attr_DW_FORM_flag_false), 1));
>> -  if (Version >= 4)
>> +  if (Version >= 4) {
>>      EXPECT_EQ(1ULL, toUnsigned(DieDG.find(Attr_DW_FORM_flag_present),
>> 0));
>> +  }
>>
>>
>>  //----------------------------------------------------------------------
>>    // Test SLEB128 based forms
>>
>>  //----------------------------------------------------------------------
>>    EXPECT_EQ(SData, toSigned(DieDG.find(Attr_DW_FORM_sdata), 0));
>> -  if (Version >= 5)
>> +  if (Version >= 5) {
>>      EXPECT_EQ(ICSData, toSigned(DieDG.find(Attr_DW_FORM_implicit_const),
>> 0));
>> +  }
>>
>>
>>  //----------------------------------------------------------------------
>>    // Test ULEB128 based forms
>> @@ -340,9 +343,10 @@ void TestAllForms() {
>>
>>  //----------------------------------------------------------------------
>>    EXPECT_EQ(Dwarf32Values[0],
>>              toReference(DieDG.find(Attr_DW_FORM_GNU_ref_alt), 0));
>> -  if (Version >= 4)
>> +  if (Version >= 4) {
>>      EXPECT_EQ(Dwarf32Values[1],
>>                toSectionOffset(DieDG.find(Attr_DW_FORM_sec_offset), 0));
>> +  }
>>
>>
>>  //----------------------------------------------------------------------
>>    // Add an address at the end to make sure we can decode this value
>>
>> Modified: llvm/trunk/unittests/Support/CommandLineTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CommandLineTest.cpp?rev=305506&r1=305505&r2=305506&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/Support/CommandLineTest.cpp (original)
>> +++ llvm/trunk/unittests/Support/CommandLineTest.cpp Thu Jun 15 16:00:40
>> 2017
>> @@ -180,8 +180,9 @@ void testCommandLineTokenizer(ParserFunc
>>    parse(Input, Saver, Actual, /*MarkEOLs=*/false);
>>    EXPECT_EQ(OutputSize, Actual.size());
>>    for (unsigned I = 0, E = Actual.size(); I != E; ++I) {
>> -    if (I < OutputSize)
>> +    if (I < OutputSize) {
>>        EXPECT_STREQ(Output[I], Actual[I]);
>> +    }
>>    }
>>  }
>>
>> @@ -528,8 +529,9 @@ TEST(CommandLineTest, GetRegisteredSubco
>>    EXPECT_FALSE(Opt1);
>>    EXPECT_FALSE(Opt2);
>>    for (auto *S : cl::getRegisteredSubcommands()) {
>> -    if (*S)
>> +    if (*S) {
>>        EXPECT_EQ("sc1", S->getName());
>> +    }
>>    }
>>
>>    cl::ResetAllOptionOccurrences();
>> @@ -538,8 +540,9 @@ TEST(CommandLineTest, GetRegisteredSubco
>>    EXPECT_FALSE(Opt1);
>>    EXPECT_FALSE(Opt2);
>>    for (auto *S : cl::getRegisteredSubcommands()) {
>> -    if (*S)
>> +    if (*S) {
>>        EXPECT_EQ("sc2", S->getName());
>> +    }
>>    }
>>  }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170619/ab57f6b5/attachment.html>


More information about the llvm-commits mailing list