Make ABSL_RETIRED_FLAG respect ABSL_FLAGS_STRIP_NAMES When ABSL_FLAGS_STRIP_NAMES is enabled, ABSL_RETIRED_FLAG should strip the flag name and avoid registering the retired flag or generating a runtime constructor. PiperOrigin-RevId: 989327332 Change-Id: I4fd128a40053395f2e2253b28232708c168b5467
diff --git a/absl/flags/flag.h b/absl/flags/flag.h index 2a95049..2e9a9e6 100644 --- a/absl/flags/flag.h +++ b/absl/flags/flag.h
@@ -298,10 +298,20 @@ // `default_value` and `explanation` are unused. // TODO(rogeeff): replace RETIRED_FLAGS with FLAGS once forward declarations of // retired flags are cleaned up. +#if ABSL_FLAGS_STRIP_NAMES +// When flag names are stripped, retired flags are not registered. Define an +// unused type alias instead of an object so that no runtime constructor or +// storage is emitted, the trailing semicolon is consumed, and `type` (along +// with this header) remains referenced to avoid unused-type/include warnings. +#define ABSL_RETIRED_FLAG(type, name, default_value, explanation) \ + using RETIRED_FLAGS_##name [[maybe_unused]] = \ + ::absl::flags_internal::RetiredFlag<type> +#else #define ABSL_RETIRED_FLAG(type, name, default_value, explanation) \ static absl::flags_internal::RetiredFlag<type> RETIRED_FLAGS_##name; \ [[maybe_unused]] static const auto RETIRED_FLAGS_REG_##name = \ (RETIRED_FLAGS_##name.Retire(#name), \ ::absl::flags_internal::FlagRegistrarEmpty{}) +#endif #endif // ABSL_FLAGS_FLAG_H_
diff --git a/absl/flags/flag_test.cc b/absl/flags/flag_test.cc index 7cbce88..458f1b1 100644 --- a/absl/flags/flag_test.cc +++ b/absl/flags/flag_test.cc
@@ -51,6 +51,7 @@ namespace { namespace flags = absl::flags_internal; +constexpr bool kStrippedFlagNames = ABSL_FLAGS_STRIP_NAMES != 0; std::string TestHelpMsg() { return "dynamic help"; } #if defined(_MSC_VER) && !defined(__clang__) @@ -230,9 +231,9 @@ namespace { TEST_F(FlagTest, TestFlagDeclaration) { -#if ABSL_FLAGS_STRIP_NAMES - GTEST_SKIP() << "This test requires flag names to be present"; -#endif + if constexpr (kStrippedFlagNames) { + GTEST_SKIP() << "This test requires flag names to be present"; + } // test that we can access flag objects. EXPECT_EQ(absl::GetFlagReflectionHandle(FLAGS_test_flag_01).Name(), "test_flag_01"); @@ -266,7 +267,7 @@ } // namespace -#if ABSL_FLAGS_STRIP_NAMES +#if ABSL_FLAGS_STRIP_NAMES && defined(__cpp_consteval) // The intent of this helper struct and an expression below is to make sure that // in the configuration where ABSL_FLAGS_STRIP_NAMES=1 registrar construction // (in cases of no Tail calls like OnUpdate) is constexpr and thus can and @@ -280,7 +281,7 @@ }; ABSL_FLAG(int, test_registrar_const_init, 0, "") + VerifyConsteval(); -#endif +#endif // ABSL_FLAGS_STRIP_NAMES && defined(__cpp_consteval) // -------------------------------------------------------------------- @@ -303,9 +304,9 @@ namespace { TEST_F(FlagTest, TestFlagDefinition) { -#if ABSL_FLAGS_STRIP_NAMES - GTEST_SKIP() << "This test requires flag names to be present"; -#endif + if constexpr (kStrippedFlagNames) { + GTEST_SKIP() << "This test requires flag names to be present"; + } absl::string_view expected_file_name = "absl/flags/flag_test.cc"; EXPECT_EQ(absl::GetFlagReflectionHandle(FLAGS_test_flag_01).Name(), @@ -626,9 +627,9 @@ // -------------------------------------------------------------------- TEST_F(FlagTest, TestGetViaReflection) { -#if ABSL_FLAGS_STRIP_NAMES - GTEST_SKIP() << "This test requires flag names to be present"; -#endif + if constexpr (kStrippedFlagNames) { + GTEST_SKIP() << "This test requires flag names to be present"; + } auto* handle = absl::FindCommandLineFlag("test_flag_01"); EXPECT_EQ(*handle->TryGet<bool>(), true); handle = absl::FindCommandLineFlag("test_flag_02"); @@ -663,9 +664,9 @@ // -------------------------------------------------------------------- TEST_F(FlagTest, ConcurrentSetAndGet) { -#if ABSL_FLAGS_STRIP_NAMES - GTEST_SKIP() << "This test requires flag names to be present"; -#endif + if constexpr (kStrippedFlagNames) { + GTEST_SKIP() << "This test requires flag names to be present"; + } static constexpr int kNumThreads = 8; // Two arbitrary durations. One thread will concurrently flip the flag // between these two values, while the other threads read it and verify @@ -816,9 +817,9 @@ using FlagDeathTest = FlagTest; TEST_F(FlagDeathTest, TestTypeMismatchValidations) { -#if ABSL_FLAGS_STRIP_NAMES - GTEST_SKIP() << "This test requires flag names to be present"; -#endif + if constexpr (kStrippedFlagNames) { + GTEST_SKIP() << "This test requires flag names to be present"; + } #if !defined(NDEBUG) EXPECT_DEATH_IF_SUPPORTED( static_cast<void>(absl::GetFlag(FLAGS_mistyped_int_flag)), @@ -950,14 +951,23 @@ TEST_F(FlagTest, TestRetiredFlagRegistration) { auto* handle = absl::FindCommandLineFlag("old_bool_flag"); - EXPECT_TRUE(handle->IsOfType<bool>()); - EXPECT_TRUE(handle->IsRetired()); - handle = absl::FindCommandLineFlag("old_int_flag"); - EXPECT_TRUE(handle->IsOfType<int>()); - EXPECT_TRUE(handle->IsRetired()); - handle = absl::FindCommandLineFlag("old_str_flag"); - EXPECT_TRUE(handle->IsOfType<std::string>()); - EXPECT_TRUE(handle->IsRetired()); + if constexpr (kStrippedFlagNames) { + EXPECT_EQ(handle, nullptr); + EXPECT_EQ(absl::FindCommandLineFlag("old_int_flag"), nullptr); + EXPECT_EQ(absl::FindCommandLineFlag("old_str_flag"), nullptr); + } else { + ASSERT_NE(handle, nullptr); + EXPECT_TRUE(handle->IsOfType<bool>()); + EXPECT_TRUE(handle->IsRetired()); + handle = absl::FindCommandLineFlag("old_int_flag"); + ASSERT_NE(handle, nullptr); + EXPECT_TRUE(handle->IsOfType<int>()); + EXPECT_TRUE(handle->IsRetired()); + handle = absl::FindCommandLineFlag("old_str_flag"); + ASSERT_NE(handle, nullptr); + EXPECT_TRUE(handle->IsOfType<std::string>()); + EXPECT_TRUE(handle->IsRetired()); + } } } // namespace @@ -1069,7 +1079,7 @@ absl::GetFlag(FLAGS_test_flag_ntc_udt1); EXPECT_EQ(value.c, 'A'); }, - 0); + 0, kStrippedFlagNames ? 1 : 0); TestExpectedLeaks<1>( [&] { @@ -1090,6 +1100,9 @@ } TEST_F(FlagTest, TestNonTriviallyCopyableParseSet) { + if constexpr (kStrippedFlagNames) { + GTEST_SKIP() << "This test requires flag names to be present"; + } TestExpectedLeaks<2>( [&] { const char* in_argv[] = {"testbin", "--test_flag_ntc_udt2=A"}; @@ -1115,7 +1128,7 @@ absl::SetFlag(&FLAGS_test_flag_ntc_udt3, value); EXPECT_EQ(value.c, 'B'); }, - 0); + 0, kStrippedFlagNames ? 1 : 0); } // One new instance created during initialization and stored in the flag. @@ -1125,6 +1138,9 @@ false); TEST_F(FlagTest, TestNonTriviallyCopyableGetBeforeMainParseGet) { + if constexpr (kStrippedFlagNames) { + GTEST_SKIP() << "This test requires flag names to be present"; + } TestExpectedLeaks<4>( [&] { const char* in_argv[] = {"testbin", "--test_flag_ntc_udt4=C"}; @@ -1153,6 +1169,9 @@ false); TEST_F(FlagTest, TestNonTriviallyCopyableSetParseGet) { + if constexpr (kStrippedFlagNames) { + GTEST_SKIP() << "This test requires flag names to be present"; + } TestExpectedLeaks<5>( [&] { const char* in_argv[] = {"testbin", "--test_flag_ntc_udt5=C"};