Skip to content

GTEST_AMBIGUOUS_ELSE_BLOCKER_ does not block the warning for gcc-7.1. #1119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gkistanova opened this issue Jun 14, 2017 · 5 comments
Closed

Comments

@gkistanova
Copy link

Starting from gcc 7.1 the GTEST_ASSERT_ and all the dependencies produce the following warning,
when used with if statement without surrounding braces.

It seems GTEST_AMBIGUOUS_ELSE_BLOCKER_ does not do the trick any more.

To reproduce, compile the following code

// gtest-ambiguous-else-warning.cpp
#include "gtest/gtest.h"

void foo(int i) {
  if (i > 0)
    GTEST_ASSERT_(::testing::AssertionFailure(), GTEST_NONFATAL_FAILURE_);
}

with

gcc-7.1 -Wall -c ./gtest-ambiguous-else-warning.cpp

The printout would be

./gtest-ambiguous-else-warning.cpp: In function 'void foo(int)':
./gtest-ambiguous-else-warning:4:6: warning: suggest explicit braces to avoid ambiguous 'else' [-Wdangling-else]
   if (i > 0)
      ^

We observe the issue with Google Test 1.8.0, but as far as I can tell, it is the same in the current trunk.

@Bu11etmagnet
Copy link

Bu11etmagnet commented Jun 27, 2017

I ran into this too (with gtest 1.7.0). This may be a bug in GCC. if () EXPECT_EQ() becomes something similar to

int main(int argc, char const *argv[])
{
  if (argc > 0) switch(0) case 0: default:
    if (argc > 1)
      return 2;
    else
      return 1;

  return 0;
}

after preprocessing and it seems pretty clear to me that the else cannot belong to the outside if.
clang correctly suppresses the warning.

@Bu11etmagnet
Copy link

pranavk added a commit to pranavk/peloton that referenced this issue Dec 16, 2017
google/googletest#1119

Let's use the braces till the problem is fixed. No harm in using extra
two braces.
pranavk added a commit to pranavk/peloton that referenced this issue Dec 16, 2017
google/googletest#1119

Let's use the braces till the problem is fixed. No harm in using extra
two braces.
pranavk added a commit to pranavk/peloton that referenced this issue Dec 20, 2017
google/googletest#1119

Let's use the braces till the problem is fixed. No harm in using extra
two braces.
apavlo pushed a commit to cmu-db/peloton that referenced this issue Dec 22, 2017
google/googletest#1119

Let's use the braces till the problem is fixed. No harm in using extra
two braces.
timarmstrong added a commit to timarmstrong/impala that referenced this issue Apr 9, 2018
Fix some ambiguous else warnings resulting from gtest macros. See
google/googletest#1119.

Add a missing include that broke compilation on the release build.

Make sure to return something from functions, even where there is
a DCHECK(false) to prevent compiler warning.

Fix alignment of aggregate function intermediate values.
@gennadiycivil
Copy link
Contributor

I have been cleaning up older and inactive GitHub googletest issues. You may see an issue "closed" if it appears to be inactive/abandoned
Thank you

asfgit pushed a commit to apache/impala that referenced this issue Jan 3, 2019
I tried compiling with GCC7 to see what warnings popped up.

Fix some ambiguous else warnings resulting from gtest macros. See
google/googletest#1119.

Add a missing include that broke compilation on the release build.

Fix some warnings that detect missing returns when there is a DCHECK
(these warnings already occurred in release builds, but they now
happen in gcc7 debug builds).

Change-Id: I39a12bc5ed6957c147b7f0dba85c7687cc989439
Reviewed-on: http://gerrit.cloudera.org:8080/12132
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
cloudera-hudson pushed a commit to cloudera/Impala that referenced this issue Mar 29, 2019
I tried compiling with GCC7 to see what warnings popped up.

Fix some ambiguous else warnings resulting from gtest macros. See
google/googletest#1119.

Add a missing include that broke compilation on the release build.

Fix some warnings that detect missing returns when there is a DCHECK
(these warnings already occurred in release builds, but they now
happen in gcc7 debug builds).

Change-Id: I39a12bc5ed6957c147b7f0dba85c7687cc989439
Reviewed-on: http://gerrit.cloudera.org:8080/12132
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
mtunique pushed a commit to mtunique/peloton that referenced this issue Apr 16, 2019
google/googletest#1119

Let's use the braces till the problem is fixed. No harm in using extra
two braces.
@rnk
Copy link

rnk commented Apr 24, 2019

Can we reopen this? This is still affecting users. See all the commits referencing this. Another user ran into this issue in LLVM:
https://reviews.llvm.org/D61046#inline-541366

llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue May 28, 2019
dtzWill pushed a commit to llvm-mirror/lldb that referenced this issue May 28, 2019
@ecatmur
Copy link

ecatmur commented Aug 27, 2019

Note googletest has disabled this warning internally: #1426

ecatmur added a commit to ecatmur/googletest that referenced this issue Aug 27, 2019
with for loops and (where necessary) lambdas.

Fixes google#1119. Requires C++11.

For example, replace:

    if (::testing::internal::IsTrue(condition))
      ;
    else
      GTEST_LOG_(FATAL) << "Condition " #condition " failed. "

with:

    for (bool gtest_check_ = ::testing::internal::IsTrue(condition); \
        !gtest_check_; gtest_check_ = true) \
      GTEST_LOG_(FATAL) << "Condition " #condition " failed. "

This allows the user to continue to stream more information into the
violation message, and avoids the "dangling else" warning-error since we
aren't using any if/else statements.

If the predicate or any branches are more complicated than a single
statement-expression, use a lambda; this requires C++11. This is
necessary for death, throw, no-throw, and no-fatal-failure tests.

Remove the -Werror=dangling-else suppression, since it isn't required
any more.
rnk added a commit to llvm/llvm-project that referenced this issue Jan 6, 2021
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
mikhailramalho added a commit to mikhailramalho/llvm-project that referenced this issue Sep 29, 2023
Explicit braces were added to fix the "suggest explicit braces to avoid
ambiguous ‘else’" warning since the current solution
(switch (0) case 0: default:) doesn't work since gcc 7
(see google/googletest#1119)

gcc 13 generates about 5000 of this warning when building libc without
this patch.
mikhailramalho added a commit to llvm/llvm-project that referenced this issue Oct 4, 2023
…7833)

Explicit braces were added to fix the "suggest explicit braces to avoid
ambiguous ‘else’" warning since the current solution (switch (0) case 0:
default:) doesn't work since gcc 7 (see
google/googletest#1119)

gcc 13 generates about 5000 of these warnings when building libc without
this patch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants