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

[Galina Kistanova via llvm-commits]

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