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

[Craig Topper via llvm-commits]

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/f80481c3/attachment.html>