[llvm] r304083 - Added braces to address gcc warning: suggest explicit braces to avoid ambiguous 'else' [-Wdangling-else]. NFC.

[Galina Kistanova via llvm-commits]

Thanks, Craid!

gcc version is 7.1.0

Here is the direct link to the builder with all the warnings, including
these, just in case you would need to know how everything has been
configured and built:
http://lab.llvm.org:8014/builders/ubuntu-gcc7.1-werror/builds/299/steps/build-unified-tree/logs/stdio


On Sat, May 27, 2017 at 10:46 PM, Craig Topper <craig.topper at gmail.com>
wrote:

> Galina, what version of gcc are you seeing this with so I can try to
> independently reproduce this?
>
> ~Craig
>
> On Sat, May 27, 2017 at 10:44 PM, Galina Kistanova <gkistanova at gmail.com>
> wrote:
>
>> It does. Here is the dependency chain:
>>
>> EXPECT_EQ -> EXPECT_PRED_FORMAT2 -> EXPECT_PRED_FORMAT2 ->
>> GTEST_PRED_FORMAT2_ -> GTEST_ASSERT_
>>
>> and GTEST_ASSERT_ has the GTEST_AMBIGUOUS_ELSE_BLOCKER_ used like this:
>>
>> #define GTEST_ASSERT_(expression, on_failure) \
>>   GTEST_AMBIGUOUS_ELSE_BLOCKER_ \
>>   if (const ::testing::AssertionResult gtest_ar = (expression)) \
>>     ; \
>>   else \
>>     on_failure(gtest_ar.failure_message())
>>
>> So, it is either used not correctly, or it does not prevent the warning.
>>
>> Anyway, I'll revert this patch for now.
>>
>>
>> On Sat, May 27, 2017 at 10:10 PM, Craig Topper <craig.topper at gmail.com>
>> wrote:
>>
>>> gtest has this define that's supposed to prevent this. You have to trace
>>> through several layers of macros but it should be used by EXPECT_EQ
>>>
>>> #ifdef __INTEL_COMPILER
>>> # define GTEST_AMBIGUOUS_ELSE_BLOCKER_
>>> #else
>>> # define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default:  //
>>> NOLINT
>>> #endif
>>>
>>> ~Craig
>>>
>>> On Sat, May 27, 2017 at 10:07 PM, Galina Kistanova via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> I did not want to fix googletest at this point. But if I'll end up
>>>> doing so, I'll fix this one too.
>>>>
>>>>
>>>> On Sat, May 27, 2017 at 9:39 PM, Davide Italiano <davide at freebsd.org>
>>>> wrote:
>>>>
>>>>> On Sat, May 27, 2017 at 8:50 PM, Galina Kistanova via llvm-commits
>>>>> <llvm-commits at lists.llvm.org> wrote:
>>>>> > Author: gkistanova
>>>>> > Date: Sat May 27 22:50:52 2017
>>>>> > New Revision: 304083
>>>>> >
>>>>> > URL: http://llvm.org/viewvc/llvm-project?rev=304083&view=rev
>>>>> > Log:
>>>>> > Added braces to address gcc warning: suggest explicit braces to
>>>>> avoid ambiguous 'else' [-Wdangling-else]. NFC.
>>>>> >
>>>>> > Modified:
>>>>> >     llvm/trunk/unittests/Support/CommandLineTest.cpp
>>>>> >
>>>>> > Modified: llvm/trunk/unittests/Support/CommandLineTest.cpp
>>>>> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Sup
>>>>> port/CommandLineTest.cpp?rev=304083&r1=304082&r2=304083&view=diff
>>>>> > ============================================================
>>>>> ==================
>>>>> > --- llvm/trunk/unittests/Support/CommandLineTest.cpp (original)
>>>>> > +++ llvm/trunk/unittests/Support/CommandLineTest.cpp Sat May 27
>>>>> 22:50:52 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());
>>>>> > +    }
>>>>> >    }
>>>>> >  }
>>>>> >
>>>>>
>>>>> I'm confused. Where's the else here ? :)
>>>>> Is EXPECT_EQ a macro that gets expanded to something? If so, maybe you
>>>>> should consider fixing it there instead of all the uses?
>>>>>
>>>>> Thanks!
>>>>>
>>>>> --
>>>>> Davide
>>>>>
>>>>> "There are no solved problems; there are only problems that are more
>>>>> or less solved" -- Henri Poincare
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> 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/20170527/df018a79/attachment.html>