Skip to content
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
merged 1 commit into from
Oct 4, 2023

Conversation

mikhailramalho
Copy link
Member

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.

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.
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 29, 2023

@llvm/pr-subscribers-libc

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/67833.diff

5 Files Affected:

  • (modified) libc/test/UnitTest/FPMatcher.h (+8-4)
  • (modified) libc/test/UnitTest/LibcTest.h (-14)
  • (modified) libc/test/src/__support/FPUtil/rounding_mode_test.cpp (+40-20)
  • (modified) libc/test/src/spawn/posix_spawn_file_actions_test.cpp (+6-3)
  • (modified) libc/utils/MPFRWrapper/MPFRUtils.h (+16-8)
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

@mikhailramalho mikhailramalho merged commit 714b4c8 into llvm:main Oct 4, 2023
4 checks passed
@mikhailramalho mikhailramalho deleted the libc-fix-dangling-else branch October 4, 2023 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants