-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
[libc][NFC] Fix -Wdangling-else when compiling libc with gcc >= 7 #67833
Merged
mikhailramalho
merged 1 commit into
llvm:main
from
mikhailramalho:libc-fix-dangling-else
Oct 4, 2023
Merged
[libc][NFC] Fix -Wdangling-else when compiling libc with gcc >= 7 #67833
mikhailramalho
merged 1 commit into
llvm:main
from
mikhailramalho:libc-fix-dangling-else
Oct 4, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
requested review from
gchatelet,
sivachandra and
michaelrj-google
September 29, 2023 16:41
@llvm/pr-subscribers-libc ChangesExplicit 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. Full diff: https://github.com/llvm/llvm-project/pull/67833.diff 5 Files Affected:
diff --git a/libc/test/UnitTest/FPMatcher.h b/libc/test/UnitTest/FPMatcher.h
index 0c1f340449236e2..fb60916a0402f05 100644
--- a/libc/test/UnitTest/FPMatcher.h
+++ b/libc/test/UnitTest/FPMatcher.h
@@ -156,17 +156,21 @@ template <TestCond C, typename T> FPMatcher<T, C> getMatcher(T expectedValue) {
do { \
using namespace LIBC_NAMESPACE::fputil::testing; \
ForceRoundingMode __r1(RoundingMode::Nearest); \
- if (__r1.success) \
+ if (__r1.success) { \
EXPECT_FP_EQ((expected), (actual)); \
+ } \
ForceRoundingMode __r2(RoundingMode::Upward); \
- if (__r2.success) \
+ if (__r2.success) { \
EXPECT_FP_EQ((expected), (actual)); \
+ } \
ForceRoundingMode __r3(RoundingMode::Downward); \
- if (__r3.success) \
+ if (__r3.success) { \
EXPECT_FP_EQ((expected), (actual)); \
+ } \
ForceRoundingMode __r4(RoundingMode::TowardZero); \
- if (__r4.success) \
+ if (__r4.success) { \
EXPECT_FP_EQ((expected), (actual)); \
+ } \
} while (0)
#endif // LLVM_LIBC_UTILS_UNITTEST_FPMATCHER_H
diff --git a/libc/test/UnitTest/LibcTest.h b/libc/test/UnitTest/LibcTest.h
index dc17c3c49d4277e..68fd5f28f3e0bd7 100644
--- a/libc/test/UnitTest/LibcTest.h
+++ b/libc/test/UnitTest/LibcTest.h
@@ -378,19 +378,6 @@ CString libc_make_test_file_path_func(const char *file_name);
SuiteClass##_##TestName SuiteClass##_##TestName##_Instance; \
void SuiteClass##_##TestName::Run()
-// 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 LIBC_AMBIGUOUS_ELSE_BLOCKER_ \
- switch (0) \
- case 0: \
- default:
-
// If RET_OR_EMPTY is the 'return' keyword we perform an early return which
// corresponds to an assert. If it is empty the execution continues, this
// corresponds to an expect.
@@ -402,7 +389,6 @@ CString libc_make_test_file_path_func(const char *file_name);
// returning a boolean. This expression is responsible for logging the
// diagnostic in case of failure.
#define LIBC_TEST_SCAFFOLDING_(TEST, RET_OR_EMPTY) \
- LIBC_AMBIGUOUS_ELSE_BLOCKER_ \
if (TEST) \
; \
else \
diff --git a/libc/test/src/__support/FPUtil/rounding_mode_test.cpp b/libc/test/src/__support/FPUtil/rounding_mode_test.cpp
index e73d10d9658841c..8077a5aab7afdec 100644
--- a/libc/test/src/__support/FPUtil/rounding_mode_test.cpp
+++ b/libc/test/src/__support/FPUtil/rounding_mode_test.cpp
@@ -19,23 +19,27 @@ TEST(LlvmLibcFEnvImplTest, QuickRoundingUpTest) {
using LIBC_NAMESPACE::fputil::fenv_is_round_up;
{
ForceRoundingMode __r(RoundingMode::Upward);
- if (__r.success)
+ if (__r.success) {
ASSERT_TRUE(fenv_is_round_up());
+ }
}
{
ForceRoundingMode __r(RoundingMode::Downward);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_up());
+ }
}
{
ForceRoundingMode __r(RoundingMode::Nearest);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_up());
+ }
}
{
ForceRoundingMode __r(RoundingMode::TowardZero);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_up());
+ }
}
}
@@ -43,23 +47,27 @@ TEST(LlvmLibcFEnvImplTest, QuickRoundingDownTest) {
using LIBC_NAMESPACE::fputil::fenv_is_round_down;
{
ForceRoundingMode __r(RoundingMode::Upward);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_down());
+ }
}
{
ForceRoundingMode __r(RoundingMode::Downward);
- if (__r.success)
+ if (__r.success) {
ASSERT_TRUE(fenv_is_round_down());
+ }
}
{
ForceRoundingMode __r(RoundingMode::Nearest);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_down());
+ }
}
{
ForceRoundingMode __r(RoundingMode::TowardZero);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_down());
+ }
}
}
@@ -67,23 +75,27 @@ TEST(LlvmLibcFEnvImplTest, QuickRoundingNearestTest) {
using LIBC_NAMESPACE::fputil::fenv_is_round_to_nearest;
{
ForceRoundingMode __r(RoundingMode::Upward);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_to_nearest());
+ }
}
{
ForceRoundingMode __r(RoundingMode::Downward);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_to_nearest());
+ }
}
{
ForceRoundingMode __r(RoundingMode::Nearest);
- if (__r.success)
+ if (__r.success) {
ASSERT_TRUE(fenv_is_round_to_nearest());
+ }
}
{
ForceRoundingMode __r(RoundingMode::TowardZero);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_to_nearest());
+ }
}
}
@@ -91,23 +103,27 @@ TEST(LlvmLibcFEnvImplTest, QuickRoundingTowardZeroTest) {
using LIBC_NAMESPACE::fputil::fenv_is_round_to_zero;
{
ForceRoundingMode __r(RoundingMode::Upward);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_to_zero());
+ }
}
{
ForceRoundingMode __r(RoundingMode::Downward);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_to_zero());
+ }
}
{
ForceRoundingMode __r(RoundingMode::Nearest);
- if (__r.success)
+ if (__r.success) {
ASSERT_FALSE(fenv_is_round_to_zero());
+ }
}
{
ForceRoundingMode __r(RoundingMode::TowardZero);
- if (__r.success)
+ if (__r.success) {
ASSERT_TRUE(fenv_is_round_to_zero());
+ }
}
}
@@ -115,22 +131,26 @@ TEST(LlvmLibcFEnvImplTest, QuickGetRoundTest) {
using LIBC_NAMESPACE::fputil::quick_get_round;
{
ForceRoundingMode __r(RoundingMode::Upward);
- if (__r.success)
+ if (__r.success) {
ASSERT_EQ(quick_get_round(), FE_UPWARD);
+ }
}
{
ForceRoundingMode __r(RoundingMode::Downward);
- if (__r.success)
+ if (__r.success) {
ASSERT_EQ(quick_get_round(), FE_DOWNWARD);
+ }
}
{
ForceRoundingMode __r(RoundingMode::Nearest);
- if (__r.success)
+ if (__r.success) {
ASSERT_EQ(quick_get_round(), FE_TONEAREST);
+ }
}
{
ForceRoundingMode __r(RoundingMode::TowardZero);
- if (__r.success)
+ if (__r.success) {
ASSERT_EQ(quick_get_round(), FE_TOWARDZERO);
+ }
}
}
diff --git a/libc/test/src/spawn/posix_spawn_file_actions_test.cpp b/libc/test/src/spawn/posix_spawn_file_actions_test.cpp
index a343710829f10b5..008167e07221f4f 100644
--- a/libc/test/src/spawn/posix_spawn_file_actions_test.cpp
+++ b/libc/test/src/spawn/posix_spawn_file_actions_test.cpp
@@ -40,12 +40,15 @@ TEST(LlvmLibcPosixSpawnFileActionsTest, AddActions) {
int action_count = 0;
while (act != nullptr) {
++action_count;
- if (action_count == 1)
+ if (action_count == 1) {
ASSERT_EQ(act->type, LIBC_NAMESPACE::BaseSpawnFileAction::CLOSE);
- if (action_count == 2)
+ }
+ if (action_count == 2) {
ASSERT_EQ(act->type, LIBC_NAMESPACE::BaseSpawnFileAction::DUP2);
- if (action_count == 3)
+ }
+ if (action_count == 3) {
ASSERT_EQ(act->type, LIBC_NAMESPACE::BaseSpawnFileAction::OPEN);
+ }
act = act->next;
}
ASSERT_EQ(action_count, 3);
diff --git a/libc/utils/MPFRWrapper/MPFRUtils.h b/libc/utils/MPFRWrapper/MPFRUtils.h
index baac2d0e3a410eb..b9573e89f25a4a9 100644
--- a/libc/utils/MPFRWrapper/MPFRUtils.h
+++ b/libc/utils/MPFRWrapper/MPFRUtils.h
@@ -356,21 +356,25 @@ template <typename T> bool round_to_long(T x, RoundingMode mode, long &result);
{ \
namespace mpfr = LIBC_NAMESPACE::testing::mpfr; \
mpfr::ForceRoundingMode __r1(mpfr::RoundingMode::Nearest); \
- if (__r1.success) \
+ if (__r1.success) { \
EXPECT_MPFR_MATCH(op, input, match_value, ulp_tolerance, \
mpfr::RoundingMode::Nearest); \
+ } \
mpfr::ForceRoundingMode __r2(mpfr::RoundingMode::Upward); \
- if (__r2.success) \
+ if (__r2.success) { \
EXPECT_MPFR_MATCH(op, input, match_value, ulp_tolerance, \
mpfr::RoundingMode::Upward); \
+ } \
mpfr::ForceRoundingMode __r3(mpfr::RoundingMode::Downward); \
- if (__r3.success) \
+ if (__r3.success) { \
EXPECT_MPFR_MATCH(op, input, match_value, ulp_tolerance, \
mpfr::RoundingMode::Downward); \
+ } \
mpfr::ForceRoundingMode __r4(mpfr::RoundingMode::TowardZero); \
- if (__r4.success) \
+ if (__r4.success) { \
EXPECT_MPFR_MATCH(op, input, match_value, ulp_tolerance, \
mpfr::RoundingMode::TowardZero); \
+ } \
}
#define TEST_MPFR_MATCH_ROUNDING_SILENTLY(op, input, match_value, \
@@ -400,21 +404,25 @@ template <typename T> bool round_to_long(T x, RoundingMode mode, long &result);
{ \
namespace mpfr = LIBC_NAMESPACE::testing::mpfr; \
mpfr::ForceRoundingMode __r1(mpfr::RoundingMode::Nearest); \
- if (__r1.success) \
+ if (__r1.success) { \
ASSERT_MPFR_MATCH(op, input, match_value, ulp_tolerance, \
mpfr::RoundingMode::Nearest); \
+ } \
mpfr::ForceRoundingMode __r2(mpfr::RoundingMode::Upward); \
- if (__r2.success) \
+ if (__r2.success) { \
ASSERT_MPFR_MATCH(op, input, match_value, ulp_tolerance, \
mpfr::RoundingMode::Upward); \
+ } \
mpfr::ForceRoundingMode __r3(mpfr::RoundingMode::Downward); \
- if (__r3.success) \
+ if (__r3.success) { \
ASSERT_MPFR_MATCH(op, input, match_value, ulp_tolerance, \
mpfr::RoundingMode::Downward); \
+ } \
mpfr::ForceRoundingMode __r4(mpfr::RoundingMode::TowardZero); \
- if (__r4.success) \
+ if (__r4.success) { \
ASSERT_MPFR_MATCH(op, input, match_value, ulp_tolerance, \
mpfr::RoundingMode::TowardZero); \
+ } \
}
#endif // LLVM_LIBC_UTILS_TESTUTILS_MPFRUTILS_H
|
lntue
approved these changes
Oct 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.