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

[Galina Kistanova via llvm-commits]

Sorry for misspelling you name, Craig.

On Sat, May 27, 2017 at 10:55 PM, Galina Kistanova <gkistanova at gmail.com>
wrote:

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