[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3
Reid Kleckner via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 24 13:57:21 PDT 2019
rnk added inline comments.
Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
> shafik wrote:
> > aganea wrote:
> > > Fixes
> > > ```
> > > [2097/2979] Building CXX object tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> > > if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> > > ^
> > > ```
> > You mixed up the error messages but I see what is going on.
> > So you may want to add a comment since it is not apparent that what is going on is due the `EXPECT_TRUE` macro eventually expanding to an `if..else` which is what is triggering the warning. Since someone may come by in the future and just remove the braces since it is not apparent why they are there.
> > Same below as well.
> EXPECT_* inside if is quite common, I don't think we should add a comment every time it is used.
This is a longstanding issue. gtest even includes this beautiful snippet of code:
// The GNU compiler emits a warning if nested "if" statements are followed by
// an "else" statement and braces are not used to explicitly disambiguate the
// "else" binding. This leads to problems with code like:
// if (gate)
// ASSERT_*(condition) << "Some message";
// The "switch (0) case 0:" idiom is used to suppress this.
# define GTEST_AMBIGUOUS_ELSE_BLOCKER_
# define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default: // NOLINT
The expansion looks something like this:
switch (0) case 0: default:
if (const TestResult res = ...);
else fail(res, ...) << "User streaming can follow"
It seems new versions of GCC have gotten "smarter" and this pattern no longer suppresses the warning as desired. It might be worth disabling -Wdangling-else for GCC at this point, since this will be an ongoing problem because LLVM typically elides braces when possible.
Oh, we even reported it back in 2017:
... and it was closed as inactive. Woohoo. :(
CHANGES SINCE LAST ACTION
More information about the cfe-commits