From 2d2a8aea291c4877e75c7991a61e57d7e12b5373 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 18 Mar 2020 01:31:55 -0700 Subject: [PATCH] Export of internal Abseil changes -- a85860e450815776c8753f34348f41ab0e918d36 by Gennadiy Rozental : Rename ComamndLineFlag's interface SetFromString as ParseFrom. This is the name approved by C++ API review. PiperOrigin-RevId: 301543521 -- 28f31bae2a136854fd89f0a32f281d12a40f702c by Abseil Team : Internal change PiperOrigin-RevId: 301524919 -- a68da3d6fbbca4c5f41a20e99ed72bb1c5dd1165 by Abseil Team : Introduce absl::base_internal::StrError, a portability wrapper around the various thread-safe alternatives to C89's strerror. PiperOrigin-RevId: 301513962 -- 92ccac3b6eb18cb41cddedbfdab53b9ad481505d by Andy Getzendanner : Introduce absl::base_internal::StrError, a portability wrapper around the various thread-safe alternatives to C89's strerror. PiperOrigin-RevId: 301493389 -- 8e196def47c250941202840d6a1de686d681cd3e by Abseil Team : Internal change PiperOrigin-RevId: 301363394 -- dd1dcffa6c47675ba4d198730a301bd408142298 by Gennadiy Rozental : Add validation for size of void* to match the size of free function pointer. PiperOrigin-RevId: 301341331 GitOrigin-RevId: a85860e450815776c8753f34348f41ab0e918d36 Change-Id: I27c9d1365d3c9b4328d1587ab3ac38e2d09a6ec2 --- CMake/AbseilDll.cmake | 2 + absl/base/BUILD.bazel | 45 +++++++++++ absl/base/CMakeLists.txt | 32 ++++++++ absl/base/internal/errno_saver_test.cc | 3 +- absl/base/internal/strerror.cc | 75 ++++++++++++++++++ absl/base/internal/strerror.h | 39 ++++++++++ absl/base/internal/strerror_benchmark.cc | 38 +++++++++ absl/base/internal/strerror_test.cc | 86 +++++++++++++++++++++ absl/flags/internal/commandlineflag.h | 10 +-- absl/flags/internal/commandlineflag_test.cc | 66 ++++++++-------- absl/flags/internal/flag.cc | 4 +- absl/flags/internal/flag.h | 23 ++++-- absl/flags/internal/registry.cc | 4 +- absl/flags/internal/type_erased.cc | 2 +- absl/flags/parse.cc | 2 +- 15 files changed, 379 insertions(+), 52 deletions(-) create mode 100644 absl/base/internal/strerror.cc create mode 100644 absl/base/internal/strerror.h create mode 100644 absl/base/internal/strerror_benchmark.cc create mode 100644 absl/base/internal/strerror_test.cc diff --git a/CMake/AbseilDll.cmake b/CMake/AbseilDll.cmake index 90c9f1f64..ed01a48d3 100644 --- a/CMake/AbseilDll.cmake +++ b/CMake/AbseilDll.cmake @@ -35,6 +35,8 @@ set(ABSL_INTERNAL_DLL_FILES "base/internal/scheduling_mode.h" "base/internal/scoped_set_env.cc" "base/internal/scoped_set_env.h" + "base/internal/strerror.h" + "base/internal/strerror.cc" "base/internal/spinlock.cc" "base/internal/spinlock.h" "base/internal/spinlock_wait.cc" diff --git a/absl/base/BUILD.bazel b/absl/base/BUILD.bazel index bae79427a..24dab7910 100644 --- a/absl/base/BUILD.bazel +++ b/absl/base/BUILD.bazel @@ -307,6 +307,7 @@ cc_test( linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ ":errno_saver", + ":strerror", "@com_google_googletest//:gtest_main", ], ) @@ -451,6 +452,7 @@ cc_binary( testonly = 1, copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, + tags = ["benchmark"], visibility = ["//visibility:private"], deps = [ ":spinlock_benchmark_common", @@ -705,3 +707,46 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_library( + name = "strerror", + srcs = ["internal/strerror.cc"], + hdrs = ["internal/strerror.h"], + copts = ABSL_DEFAULT_COPTS, + linkopts = ABSL_DEFAULT_LINKOPTS, + visibility = [ + "//absl:__subpackages__", + ], + deps = [ + ":config", + ":core_headers", + ":errno_saver", + ], +) + +cc_test( + name = "strerror_test", + size = "small", + srcs = ["internal/strerror_test.cc"], + copts = ABSL_TEST_COPTS, + linkopts = ABSL_DEFAULT_LINKOPTS, + deps = [ + ":strerror", + "//absl/strings", + "@com_google_googletest//:gtest_main", + ], +) + +cc_binary( + name = "strerror_benchmark", + testonly = 1, + srcs = ["internal/strerror_benchmark.cc"], + copts = ABSL_TEST_COPTS, + linkopts = ABSL_DEFAULT_LINKOPTS, + tags = ["benchmark"], + visibility = ["//visibility:private"], + deps = [ + ":strerror", + "@com_github_google_benchmark//:benchmark_main", + ], +) diff --git a/absl/base/CMakeLists.txt b/absl/base/CMakeLists.txt index 14c52eabd..4230d2e73 100644 --- a/absl/base/CMakeLists.txt +++ b/absl/base/CMakeLists.txt @@ -326,6 +326,7 @@ absl_cc_test( ${ABSL_TEST_COPTS} DEPS absl::errno_saver + absl::strerror gmock gtest_main ) @@ -642,3 +643,34 @@ absl_cc_test( gmock gtest_main ) + +absl_cc_library( + NAME + strerror + SRCS + "internal/strerror.cc" + HDRS + "internal/strerror.h" + COPTS + ${ABSL_DEFAULT_COPTS} + LINKOPTS + ${ABSL_DEFAULT_LINKOPTS} + DEPS + absl::config + absl::core_headers + absl::errno_saver +) + +absl_cc_test( + NAME + strerror_test + SRCS + "internal/strerror_test.cc" + COPTS + ${ABSL_TEST_COPTS} + DEPS + absl::strerror + absl::strings + gmock + gtest_main +) diff --git a/absl/base/internal/errno_saver_test.cc b/absl/base/internal/errno_saver_test.cc index b845e2dd1..e9b742c58 100644 --- a/absl/base/internal/errno_saver_test.cc +++ b/absl/base/internal/errno_saver_test.cc @@ -18,6 +18,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "absl/base/internal/strerror.h" namespace { using ::testing::Eq; @@ -26,7 +27,7 @@ struct ErrnoPrinter { int no; }; std::ostream &operator<<(std::ostream &os, ErrnoPrinter ep) { - return os << strerror(ep.no) << " [" << ep.no << "]"; + return os << absl::base_internal::StrError(ep.no) << " [" << ep.no << "]"; } bool operator==(ErrnoPrinter one, ErrnoPrinter two) { return one.no == two.no; } diff --git a/absl/base/internal/strerror.cc b/absl/base/internal/strerror.cc new file mode 100644 index 000000000..af181513c --- /dev/null +++ b/absl/base/internal/strerror.cc @@ -0,0 +1,75 @@ +// Copyright 2020 The Abseil Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "absl/base/internal/strerror.h" + +#include +#include +#include +#include +#include +#include + +#include "absl/base/attributes.h" +#include "absl/base/internal/errno_saver.h" + +namespace absl { +ABSL_NAMESPACE_BEGIN +namespace base_internal { +namespace { +const char* StrErrorAdaptor(int errnum, char* buf, size_t buflen) { +#if defined(_WIN32) + int rc = strerror_s(buf, buflen, errnum); + buf[buflen - 1] = '\0'; // guarantee NUL termination + if (rc == 0 && strncmp(buf, "Unknown error", buflen) == 0) *buf = '\0'; + return buf; +#else +#if defined(__GLIBC__) || defined(__APPLE__) + // Use the BSD sys_errlist API provided by GNU glibc and others to + // avoid any need to copy the message into the local buffer first. + if (0 <= errnum && errnum < sys_nerr) { + if (const char* p = sys_errlist[errnum]) { + return p; + } + } +#endif + // The type of `ret` is platform-specific; both of these branches must compile + // either way but only one will execute on any given platform: + auto ret = strerror_r(errnum, buf, buflen); + if (std::is_same::value) { + // XSI `strerror_r`; `ret` is `int`: + if (ret) *buf = '\0'; + return buf; + } else { + // GNU `strerror_r`; `ret` is `char *`: + return reinterpret_cast(ret); + } +#endif +} +} // namespace + +std::string StrError(int errnum) { + absl::base_internal::ErrnoSaver errno_saver; + char buf[100]; + const char* str = StrErrorAdaptor(errnum, buf, sizeof buf); + if (*str == '\0') { + snprintf(buf, sizeof buf, "Unknown error %d", errnum); + str = buf; + } + return str; +} + +} // namespace base_internal +ABSL_NAMESPACE_END +} // namespace absl diff --git a/absl/base/internal/strerror.h b/absl/base/internal/strerror.h new file mode 100644 index 000000000..350097366 --- /dev/null +++ b/absl/base/internal/strerror.h @@ -0,0 +1,39 @@ +// Copyright 2020 The Abseil Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef ABSL_BASE_INTERNAL_STRERROR_H_ +#define ABSL_BASE_INTERNAL_STRERROR_H_ + +#include + +#include "absl/base/config.h" + +namespace absl { +ABSL_NAMESPACE_BEGIN +namespace base_internal { + +// A portable and thread-safe alternative to C89's `strerror`. +// +// The C89 specification of `strerror` is not suitable for use in a +// multi-threaded application as the returned string may be changed by calls to +// `strerror` from another thread. The many non-stdlib alternatives differ +// enough in their names, availability, and semantics to justify this wrapper +// around them. `errno` will not be modified by a call to `absl::StrError`. +std::string StrError(int errnum); + +} // namespace base_internal +ABSL_NAMESPACE_END +} // namespace absl + +#endif // ABSL_BASE_INTERNAL_STRERROR_H_ diff --git a/absl/base/internal/strerror_benchmark.cc b/absl/base/internal/strerror_benchmark.cc new file mode 100644 index 000000000..d8ca86b95 --- /dev/null +++ b/absl/base/internal/strerror_benchmark.cc @@ -0,0 +1,38 @@ +// Copyright 2020 The Abseil Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include + +#include "absl/base/internal/strerror.h" +#include "benchmark/benchmark.h" + +namespace { +#if defined(__GLIBC__) || defined(__APPLE__) +void BM_SysErrList(benchmark::State& state) { + for (auto _ : state) { + benchmark::DoNotOptimize(std::string(sys_errlist[ERANGE])); + } +} +BENCHMARK(BM_SysErrList); +#endif + +void BM_AbslStrError(benchmark::State& state) { + for (auto _ : state) { + benchmark::DoNotOptimize(absl::base_internal::StrError(ERANGE)); + } +} +BENCHMARK(BM_AbslStrError); +} // namespace diff --git a/absl/base/internal/strerror_test.cc b/absl/base/internal/strerror_test.cc new file mode 100644 index 000000000..a53da97f9 --- /dev/null +++ b/absl/base/internal/strerror_test.cc @@ -0,0 +1,86 @@ +// Copyright 2020 The Abseil Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "absl/base/internal/strerror.h" + +#include +#include +#include +#include +#include +#include // NOLINT(build/c++11) +#include + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "absl/strings/match.h" + +namespace { +using ::testing::AnyOf; +using ::testing::Eq; + +TEST(StrErrorTest, ValidErrorCode) { + errno = ERANGE; + EXPECT_THAT(absl::base_internal::StrError(EDOM), Eq(strerror(EDOM))); + EXPECT_THAT(errno, Eq(ERANGE)); +} + +TEST(StrErrorTest, InvalidErrorCode) { + errno = ERANGE; + EXPECT_THAT(absl::base_internal::StrError(-1), + AnyOf(Eq("No error information"), Eq("Unknown error -1"))); + EXPECT_THAT(errno, Eq(ERANGE)); +} + +TEST(StrErrorTest, MultipleThreads) { + // In this test, we will start up 2 threads and have each one call + // StrError 1000 times, each time with a different errnum. We + // expect that StrError(errnum) will return a string equal to the + // one returned by strerror(errnum), if the code is known. Since + // strerror is known to be thread-hostile, collect all the expected + // strings up front. + const int kNumCodes = 1000; + std::vector expected_strings(kNumCodes); + for (int i = 0; i < kNumCodes; ++i) { + expected_strings[i] = strerror(i); + } + + std::atomic_int counter(0); + auto thread_fun = [&]() { + for (int i = 0; i < kNumCodes; ++i) { + ++counter; + errno = ERANGE; + const std::string value = absl::base_internal::StrError(i); + // Only the GNU implementation is guaranteed to provide the + // string "Unknown error nnn". POSIX doesn't say anything. + if (!absl::StartsWith(value, "Unknown error ")) { + EXPECT_THAT(absl::base_internal::StrError(i), Eq(expected_strings[i])); + } + EXPECT_THAT(errno, Eq(ERANGE)); + } + }; + + const int kNumThreads = 100; + std::vector threads; + for (int i = 0; i < kNumThreads; ++i) { + threads.push_back(std::thread(thread_fun)); + } + for (auto& thread : threads) { + thread.join(); + } + + EXPECT_THAT(counter, Eq(kNumThreads * kNumCodes)); +} + +} // namespace diff --git a/absl/flags/internal/commandlineflag.h b/absl/flags/internal/commandlineflag.h index 9a740d574..338a1228d 100644 --- a/absl/flags/internal/commandlineflag.h +++ b/absl/flags/internal/commandlineflag.h @@ -65,7 +65,7 @@ enum FlagSettingMode { SET_FLAGS_DEFAULT }; -// Options that control SetFromString: Source of a value. +// Options that control ParseFrom: Source of a value. enum ValueSource { // Flag is being set by value specified on a command line. kCommandLine, @@ -171,10 +171,10 @@ class CommandLineFlag { // * Update the flag's default value // * Update the current flag value if it was never set before // The mode is selected based on `set_mode` parameter. - virtual bool SetFromString(absl::string_view value, - flags_internal::FlagSettingMode set_mode, - flags_internal::ValueSource source, - std::string* error) = 0; + virtual bool ParseFrom(absl::string_view value, + flags_internal::FlagSettingMode set_mode, + flags_internal::ValueSource source, + std::string* error) = 0; // Checks that flags default value can be converted to string and back to the // flag's value type. diff --git a/absl/flags/internal/commandlineflag_test.cc b/absl/flags/internal/commandlineflag_test.cc index 0e8bc3133..c1142b7c5 100644 --- a/absl/flags/internal/commandlineflag_test.cc +++ b/absl/flags/internal/commandlineflag_test.cc @@ -119,99 +119,99 @@ TEST_F(CommandLineFlagTest, TestValueAccessMethods) { // -------------------------------------------------------------------- -TEST_F(CommandLineFlagTest, TestSetFromStringCurrentValue) { +TEST_F(CommandLineFlagTest, TestParseFromCurrentValue) { std::string err; auto* flag_01 = flags::FindCommandLineFlag("int_flag"); EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); - EXPECT_TRUE(flag_01->SetFromString("11", flags::SET_FLAGS_VALUE, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_01->ParseFrom("11", flags::SET_FLAGS_VALUE, + flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), 11); EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); - EXPECT_TRUE(flag_01->SetFromString("-123", flags::SET_FLAGS_VALUE, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_01->ParseFrom("-123", flags::SET_FLAGS_VALUE, + flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), -123); EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); - EXPECT_TRUE(!flag_01->SetFromString("xyz", flags::SET_FLAGS_VALUE, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(!flag_01->ParseFrom("xyz", flags::SET_FLAGS_VALUE, + flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), -123); EXPECT_EQ(err, "Illegal value 'xyz' specified for flag 'int_flag'"); EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); - EXPECT_TRUE(!flag_01->SetFromString("A1", flags::SET_FLAGS_VALUE, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(!flag_01->ParseFrom("A1", flags::SET_FLAGS_VALUE, + flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), -123); EXPECT_EQ(err, "Illegal value 'A1' specified for flag 'int_flag'"); EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); - EXPECT_TRUE(flag_01->SetFromString("0x10", flags::SET_FLAGS_VALUE, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_01->ParseFrom("0x10", flags::SET_FLAGS_VALUE, + flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), 16); EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); - EXPECT_TRUE(flag_01->SetFromString("011", flags::SET_FLAGS_VALUE, - flags::kCommandLine, &err)); + EXPECT_TRUE(flag_01->ParseFrom("011", flags::SET_FLAGS_VALUE, + flags::kCommandLine, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), 11); EXPECT_TRUE(flag_01->IsSpecifiedOnCommandLine()); - EXPECT_TRUE(!flag_01->SetFromString("", flags::SET_FLAGS_VALUE, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(!flag_01->ParseFrom("", flags::SET_FLAGS_VALUE, + flags::kProgrammaticChange, &err)); EXPECT_EQ(err, "Illegal value '' specified for flag 'int_flag'"); auto* flag_02 = flags::FindCommandLineFlag("string_flag"); - EXPECT_TRUE(flag_02->SetFromString("xyz", flags::SET_FLAGS_VALUE, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_02->ParseFrom("xyz", flags::SET_FLAGS_VALUE, + flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_string_flag), "xyz"); - EXPECT_TRUE(flag_02->SetFromString("", flags::SET_FLAGS_VALUE, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_02->ParseFrom("", flags::SET_FLAGS_VALUE, + flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_string_flag), ""); } // -------------------------------------------------------------------- -TEST_F(CommandLineFlagTest, TestSetFromStringDefaultValue) { +TEST_F(CommandLineFlagTest, TestParseFromDefaultValue) { std::string err; auto* flag_01 = flags::FindCommandLineFlag("int_flag"); - EXPECT_TRUE(flag_01->SetFromString("111", flags::SET_FLAGS_DEFAULT, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_01->ParseFrom("111", flags::SET_FLAGS_DEFAULT, + flags::kProgrammaticChange, &err)); EXPECT_EQ(flag_01->DefaultValue(), "111"); auto* flag_02 = flags::FindCommandLineFlag("string_flag"); - EXPECT_TRUE(flag_02->SetFromString("abc", flags::SET_FLAGS_DEFAULT, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_02->ParseFrom("abc", flags::SET_FLAGS_DEFAULT, + flags::kProgrammaticChange, &err)); EXPECT_EQ(flag_02->DefaultValue(), "abc"); } // -------------------------------------------------------------------- -TEST_F(CommandLineFlagTest, TestSetFromStringIfDefault) { +TEST_F(CommandLineFlagTest, TestParseFromIfDefault) { std::string err; auto* flag_01 = flags::FindCommandLineFlag("int_flag"); - EXPECT_TRUE(flag_01->SetFromString("22", flags::SET_FLAG_IF_DEFAULT, - flags::kProgrammaticChange, &err)) + EXPECT_TRUE(flag_01->ParseFrom("22", flags::SET_FLAG_IF_DEFAULT, + flags::kProgrammaticChange, &err)) << err; EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), 22); - EXPECT_TRUE(flag_01->SetFromString("33", flags::SET_FLAG_IF_DEFAULT, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_01->ParseFrom("33", flags::SET_FLAG_IF_DEFAULT, + flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), 22); // EXPECT_EQ(err, "ERROR: int_flag is already set to 22"); // Reset back to default value - EXPECT_TRUE(flag_01->SetFromString("201", flags::SET_FLAGS_VALUE, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_01->ParseFrom("201", flags::SET_FLAGS_VALUE, + flags::kProgrammaticChange, &err)); - EXPECT_TRUE(flag_01->SetFromString("33", flags::SET_FLAG_IF_DEFAULT, - flags::kProgrammaticChange, &err)); + EXPECT_TRUE(flag_01->ParseFrom("33", flags::SET_FLAG_IF_DEFAULT, + flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), 201); // EXPECT_EQ(err, "ERROR: int_flag is already set to 201"); } diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc index a12fe7c5e..a45e883e0 100644 --- a/absl/flags/internal/flag.cc +++ b/absl/flags/internal/flag.cc @@ -338,8 +338,8 @@ void FlagImpl::Write(const void* src) { // * Update the flag's default value // * Update the current flag value if it was never set before // The mode is selected based on 'set_mode' parameter. -bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode, - ValueSource source, std::string* err) { +bool FlagImpl::ParseFrom(absl::string_view value, FlagSettingMode set_mode, + ValueSource source, std::string* err) { absl::MutexLock l(DataGuard()); switch (set_mode) { diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h index 0ef0ee745..2c4aba680 100644 --- a/absl/flags/internal/flag.h +++ b/absl/flags/internal/flag.h @@ -76,8 +76,17 @@ void* FlagOps(FlagOp op, const void* v1, void* v2, void* v3) { return nullptr; case FlagOp::kSizeof: return reinterpret_cast(sizeof(T)); - case FlagOp::kStaticTypeId: - return reinterpret_cast(&FlagStaticTypeIdGen); + case FlagOp::kStaticTypeId: { + auto* static_id = &FlagStaticTypeIdGen; + + // Cast from function pointer to void* is not portable. + // We don't have an easy way to work around this, but it works fine + // on all the platforms we test and as long as size of pointers match + // we should be fine to do reinterpret cast. + static_assert(sizeof(void*) == sizeof(static_id), + "Flag's static type id does not work on this platform"); + return reinterpret_cast(static_id); + } case FlagOp::kParse: { // Initialize the temporary instance of type T based on current value in // destination (which is going to be flag's default value). @@ -395,8 +404,8 @@ class FlagImpl { // Mutating access methods void Write(const void* src) ABSL_LOCKS_EXCLUDED(*DataGuard()); - bool SetFromString(absl::string_view value, FlagSettingMode set_mode, - ValueSource source, std::string* err) + bool ParseFrom(absl::string_view value, FlagSettingMode set_mode, + ValueSource source, std::string* err) ABSL_LOCKS_EXCLUDED(*DataGuard()); // Interfaces to operate on callbacks. @@ -586,9 +595,9 @@ class Flag final : public flags_internal::CommandLineFlag { return impl_.RestoreState(&flag_state.cur_value_, flag_state.modified_, flag_state.on_command_line_, flag_state.counter_); } - bool SetFromString(absl::string_view value, FlagSettingMode set_mode, - ValueSource source, std::string* error) override { - return impl_.SetFromString(value, set_mode, source, error); + bool ParseFrom(absl::string_view value, FlagSettingMode set_mode, + ValueSource source, std::string* error) override { + return impl_.ParseFrom(value, set_mode, source, error); } void CheckDefaultValueParsingRoundtrip() const override { impl_.CheckDefaultValueParsingRoundtrip(); diff --git a/absl/flags/internal/registry.cc b/absl/flags/internal/registry.cc index e434a8591..9ed912140 100644 --- a/absl/flags/internal/registry.cc +++ b/absl/flags/internal/registry.cc @@ -306,8 +306,8 @@ class RetiredFlagObj final : public flags_internal::CommandLineFlag { return nullptr; } - bool SetFromString(absl::string_view, flags_internal::FlagSettingMode, - flags_internal::ValueSource, std::string*) override { + bool ParseFrom(absl::string_view, flags_internal::FlagSettingMode, + flags_internal::ValueSource, std::string*) override { return false; } diff --git a/absl/flags/internal/type_erased.cc b/absl/flags/internal/type_erased.cc index 490bc4eba..75b4cdf89 100644 --- a/absl/flags/internal/type_erased.cc +++ b/absl/flags/internal/type_erased.cc @@ -56,7 +56,7 @@ bool SetCommandLineOptionWithMode(absl::string_view name, if (!flag || flag->IsRetired()) return false; std::string error; - if (!flag->SetFromString(value, set_mode, kProgrammaticChange, &error)) { + if (!flag->ParseFrom(value, set_mode, kProgrammaticChange, &error)) { // Errors here are all of the form: the provided name was a recognized // flag, but the value was invalid (bad type, or validation failed). flags_internal::ReportUsageError(error, false); diff --git a/absl/flags/parse.cc b/absl/flags/parse.cc index af5fb12dc..b60b36f60 100644 --- a/absl/flags/parse.cc +++ b/absl/flags/parse.cc @@ -696,7 +696,7 @@ std::vector ParseCommandLineImpl(int argc, char* argv[], if (flag->IsRetired()) continue; std::string error; - if (!flag->SetFromString(value, SET_FLAGS_VALUE, kCommandLine, &error)) { + if (!flag->ParseFrom(value, SET_FLAGS_VALUE, kCommandLine, &error)) { flags_internal::ReportUsageError(error, true); success = false; }