From 4faa33ce990cc6767b7a2f02c8eb6c2f3a573e8f Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Thu, 5 Mar 2020 14:15:47 -0500 Subject: [PATCH 1/8] cleanup: use same clang-tidy in all repositories Use the clang-tidy distributed with Fedora (currently clang-tidy-9) in this repository. I am planning to make similar changes in the other repositories and bring all our `clang-tidy` builds to use the same versions of `clang-format`, `clang-tidy`, `doxygen`, `buildifier`, `shellcheck`, and `cmake_format`. I also updated the clang-tidy configuration to match the newer repos (`-pubsub` and `-spanner`), and cleaned up any errors found in the files. Finally, fixed a number of Doxygen comments that used `` (for a template parameter) which Doxygen thought was an unordered list or something. --- .clang-tidy | 41 ++++++-- ci/kokoro/docker/Dockerfile.fedora-install | 93 +++++++++---------- ci/kokoro/docker/Dockerfile.ubuntu-install | 22 ----- ci/kokoro/docker/build-in-docker-cmake.sh | 8 +- ci/kokoro/docker/build.sh | 12 ++- google/cloud/future_generic.h | 6 +- google/cloud/future_generic_test.cc | 33 ++++--- google/cloud/future_generic_then_test.cc | 77 ++++++++------- google/cloud/future_void.h | 6 +- google/cloud/future_void_test.cc | 33 ++++--- google/cloud/future_void_then_test.cc | 77 ++++++++------- google/cloud/internal/build_info.cc.in | 3 +- google/cloud/internal/compiler_info.cc | 12 ++- google/cloud/internal/filesystem_test.cc | 29 +++--- google/cloud/internal/format_time_point.cc | 64 +++++++++---- google/cloud/internal/future_impl.h | 10 +- google/cloud/internal/future_impl_test.cc | 44 +++++---- google/cloud/internal/future_then_meta.h | 6 +- .../cloud/internal/pagination_range_test.cc | 20 ++-- google/cloud/internal/parse_rfc3339.cc | 46 +++++---- google/cloud/log.cc | 10 +- google/cloud/optional_test.cc | 2 +- google/cloud/testing_util/expect_exception.h | 11 ++- .../cloud/testing_util/expect_future_error.h | 11 ++- 24 files changed, 384 insertions(+), 292 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 0750676..561222c 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,9 +1,30 @@ --- # Configure clang-tidy for this project. -# Disabled: -# -google-readability-namespace-comments the *_CLIENT_NS is a macro, and -# clang-tidy fails to match it against the initial value. +# Here is an explanation for why some of the checks are disabled: +# +# -google-readability-namespace-comments: the *_CLIENT_NS is a macro, and +# clang-tidy fails to match it against the initial value. +# +# -modernize-use-trailing-return-type: clang-tidy recommends using +# `auto Foo() -> std::string { return ...; }`, we think the code is less +# readable in this form. +# +# -modernize-return-braced-init-list: We think removing typenames and using +# only braced-init can hurt readability. +# +# -performance-move-const-arg: This warning requires the developer to +# know/care more about the implementation details of types/functions than +# should be necessary. For example, `A a; F(std::move(a));` will trigger a +# warning IFF `A` is a trivial type (and therefore the move is +# meaningless). It would also warn if `F` accepts by `const&`, which is +# another detail that the caller need not care about. +# +# -readability-redundant-declaration: A friend declaration inside a class +# counts as a declaration, so if we also declare that friend outside the +# class in order to document it as part of the public API, that will +# trigger a redundant declaration warning from this check. +# Checks: > -*, bugprone-*, @@ -13,24 +34,32 @@ Checks: > performance-*, portability-*, readability-*, + -google-readability-braces-around-statements, -google-readability-namespace-comments, - -google-runtime-int, -google-runtime-references, -misc-non-private-member-variables-in-classes, + -modernize-return-braced-init-list, + -performance-move-const-arg, -readability-named-parameter, + -modernize-use-trailing-return-type, -readability-braces-around-statements, - -readability-magic-numbers + -readability-redundant-declaration # Turn all the warnings from the checks above into errors. WarningsAsErrors: "*" +# TODO(#...) - Enable clang-tidy checks in our headers. +# HeaderFilterRegex: "google/cloud/.*" + CheckOptions: - { key: readability-identifier-naming.NamespaceCase, value: lower_case } - { key: readability-identifier-naming.ClassCase, value: CamelCase } - { key: readability-identifier-naming.StructCase, value: CamelCase } - { key: readability-identifier-naming.TemplateParameterCase, value: CamelCase } - - { key: readability-identifier-naming.FunctionCase, value: CamelCase } + - { key: readability-identifier-naming.FunctionCase, value: aNy_CasE } - { key: readability-identifier-naming.VariableCase, value: lower_case } + - { key: readability-identifier-naming.ClassMemberCase, value: lower_case } + - { key: readability-identifier-naming.ClassMemberSuffix, value: _ } - { key: readability-identifier-naming.PrivateMemberSuffix, value: _ } - { key: readability-identifier-naming.ProtectedMemberSuffix, value: _ } - { key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE } diff --git a/ci/kokoro/docker/Dockerfile.fedora-install b/ci/kokoro/docker/Dockerfile.fedora-install index dce2f9b..ef2a2d3 100644 --- a/ci/kokoro/docker/Dockerfile.fedora-install +++ b/ci/kokoro/docker/Dockerfile.fedora-install @@ -12,62 +12,59 @@ # See the License for the specific language governing permissions and # limitations under the License. -ARG DISTRO_VERSION=30 +ARG DISTRO_VERSION=31 FROM fedora:${DISTRO_VERSION} ARG NCPU=4 -RUN dnf makecache && dnf install -y \ - autoconf \ - automake \ - c-ares-devel \ - ccache \ - clang \ - clang-analyzer \ - clang-tools-extra \ - cmake \ - curl \ - doxygen \ - gcc-c++ \ - git \ - grpc-devel \ - grpc-plugins \ - libcxx-devel \ - libcxxabi-devel \ - libtool \ - make \ - ncurses-term \ - openssl-devel \ - pkgconfig \ - protobuf-compiler \ - python3 \ - shtool \ - unzip \ - wget \ - which \ - zlib-devel +# Fedora includes packages for gRPC, libcurl, and OpenSSL that are recent enough +# for `google-cloud-cpp`. Install these packages and additional development +# tools to compile the dependencies: +RUN dnf makecache && \ + dnf install -y abi-compliance-checker abi-dumper \ + clang clang-tools-extra cmake diffutils doxygen findutils gcc-c++ git \ + grpc-devel grpc-plugins libcxx-devel libcxxabi-devel libcurl-devel \ + make openssl-devel pkgconfig protobuf-compiler python3 python3-pip \ + ShellCheck tar unzip w3m wget which zlib-devel -# Install googleapis. -WORKDIR /var/tmp/build -RUN wget -q https://github.com/googleapis/cpp-cmakefiles/archive/v0.4.1.tar.gz -RUN tar -xf v0.4.1.tar.gz -WORKDIR /var/tmp/build/cpp-cmakefiles-0.4.1 -RUN cmake \ - -DBUILD_SHARED_LIBS=YES \ - -H. -Bcmake-out -RUN cmake --build cmake-out --target install -- -j ${NCPU} -RUN ldconfig +# Install the the buildifier tool, which does not compile with the default +# golang compiler for Ubuntu 16.04 and Ubuntu 18.04. +RUN wget -q -O /usr/bin/buildifier https://github.com/bazelbuild/buildtools/releases/download/0.29.0/buildifier +RUN chmod 755 /usr/bin/buildifier -# Install googletest. +# Install cmake_format to automatically format the CMake list files. +# https://github.com/cheshirekow/cmake_format +# Pin this to an specific version because the formatting changes when the +# "latest" version is updated, and we do not want the builds to break just +# because some third party changed something. +RUN pip3 install --upgrade pip +RUN pip3 install cmake_format==0.6.8 + +# Install googletest, remove the downloaded files and the temporary artifacts +# after a successful build to keep the image smaller (and with fewer layers) WORKDIR /var/tmp/build -RUN wget -q https://github.com/google/googletest/archive/release-1.10.0.tar.gz -RUN tar -xf release-1.10.0.tar.gz -WORKDIR /var/tmp/build/googletest-release-1.10.0 -RUN cmake \ +RUN wget -q https://github.com/google/googletest/archive/release-1.10.0.tar.gz && \ + tar -xf release-1.10.0.tar.gz && \ + cd /var/tmp/build/googletest-release-1.10.0 && \ + cmake \ -DCMAKE_BUILD_TYPE="Release" \ -DBUILD_SHARED_LIBS=yes \ - -H. -Bcmake-out/googletest -RUN cmake --build cmake-out/googletest --target install -- -j ${NCPU} -RUN ldconfig + -H. -Bcmake-out/googletest && \ + cmake --build cmake-out/googletest --target install -- -j ${NCPU} && \ + ldconfig && \ + cd /var/tmp && rm -fr build + +# Install googletest, remove the downloaded files and the temporary artifacts +# after a successful build to keep the image smaller (and with fewer layers) +WORKDIR /var/tmp/build +RUN wget -q https://github.com/googleapis/cpp-cmakefiles/archive/v0.5.0.tar.gz && \ + tar -xf v0.5.0.tar.gz && \ + cd /var/tmp/build/cpp-cmakefiles-0.5.0 && \ + cmake \ + -DBUILD_SHARED_LIBS=YES \ + -H. -Bcmake-out && \ + cmake --build cmake-out --target install -- -j ${NCPU} && \ + ldconfig && \ + cd /var/tmp && rm -fr build # Install Bazel because some of the builds need it. WORKDIR /var/tmp/ci diff --git a/ci/kokoro/docker/Dockerfile.ubuntu-install b/ci/kokoro/docker/Dockerfile.ubuntu-install index e65ad8c..9872a9f 100644 --- a/ci/kokoro/docker/Dockerfile.ubuntu-install +++ b/ci/kokoro/docker/Dockerfile.ubuntu-install @@ -51,28 +51,6 @@ RUN apt-get update && \ wget \ zlib1g-dev -# By default, Ubuntu 18.04 does not install the alternatives for clang-format -# and clang-tidy, so we need to manually install those. -RUN if grep -q 18.04 /etc/lsb-release; then \ - apt-get update && apt-get --no-install-recommends install -y clang-tidy clang-format-7 clang-tools; \ - update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-6.0 100; \ - update-alternatives --install /usr/bin/clang-format clang-format /usr/bin/clang-format-7 100; \ - update-alternatives --install /usr/bin/scan-build scan-build /usr/bin/scan-build-6.0 100; \ - fi - -# Install the the buildifier tool, which does not compile with the default -# golang compiler for Ubuntu 16.04 and Ubuntu 18.04. -RUN wget -q -O /usr/bin/buildifier https://github.com/bazelbuild/buildtools/releases/download/0.29.0/buildifier -RUN chmod 755 /usr/bin/buildifier - -# Install cmake_format to automatically format the CMake list files. -# https://github.com/cheshirekow/cmake_format -# Pin this to an specific version because the formatting changes when the -# "latest" version is updated, and we do not want the builds to break just -# because some third party changed something. -RUN pip3 install --upgrade pip -RUN pip3 install setuptools cmake_format==0.6.8 - # Install protobuf using CMake. Some distributions include protobuf, but gRPC # requires 3.4.x or newer, and many of those distribution use older versions. # We need to install both the debug and Release version because: diff --git a/ci/kokoro/docker/build-in-docker-cmake.sh b/ci/kokoro/docker/build-in-docker-cmake.sh index c34ecb6..38729f5 100755 --- a/ci/kokoro/docker/build-in-docker-cmake.sh +++ b/ci/kokoro/docker/build-in-docker-cmake.sh @@ -55,15 +55,15 @@ cmake_extra_flags=( # Always set the build type, so it can be configured by the `build.sh` # script. "-DCMAKE_BUILD_TYPE=${BUILD_TYPE}" - - # Always disable the ccache, it can make some builds flaky, and we do not - # preserve the cache between builds. - "-DGOOGLE_CLOUD_CPP_ENABLE_CCACHE=OFF" ) if [[ "${BUILD_TESTING:-}" == "no" ]]; then cmake_extra_flags+=( "-DBUILD_TESTING=OFF" ) fi +if [[ "${CLANG_TIDY:-}" = "yes" ]]; then + cmake_extra_flags+=("-DGOOGLE_CLOUD_CPP_CLANG_TIDY=yes") +fi + if [[ "${GOOGLE_CLOUD_CPP_CXX_STANDARD:-}" != "" ]]; then cmake_extra_flags+=( "-DGOOGLE_CLOUD_CPP_CXX_STANDARD=${GOOGLE_CLOUD_CPP_CXX_STANDARD}") diff --git a/ci/kokoro/docker/build.sh b/ci/kokoro/docker/build.sh index 58ec93b..471a4c5 100755 --- a/ci/kokoro/docker/build.sh +++ b/ci/kokoro/docker/build.sh @@ -57,12 +57,16 @@ fi if [[ "${BUILD_NAME}" = "clang-tidy" ]]; then # Compile with clang-tidy(1) turned on. The build treats clang-tidy warnings # as errors. - export BUILD_TYPE=Debug + export DISTRO=fedora-install + export DISTRO_VERSION=31 export CC=clang export CXX=clang++ - export CMAKE_FLAGS="-DGOOGLE_CLOUD_CPP_CLANG_TIDY=yes" + export BUILD_TYPE=Debug export CHECK_STYLE=yes export GENERATE_DOCS=yes + export CLANG_TIDY=yes + export TEST_INSTALL=yes + in_docker_script="ci/kokoro/docker/build-in-docker-cmake.sh" elif [[ "${BUILD_NAME}" = "publish-refdocs" ]]; then export BUILD_TYPE=Debug export CC=clang @@ -374,6 +378,10 @@ docker_flags=( # clang-format, cmake-format, and buildifier). "--env" "CHECK_STYLE=${CHECK_STYLE:-}" + # If set to 'yes', the build script will configure clang-tidy. Currently + # only the CMake builds use this flag. + "--env" "CLANG_TIDY=${CLANG_TIDY:-}" + # If set to 'no', skip the integration tests. "--env" "RUN_INTEGRATION_TESTS=${RUN_INTEGRATION_TESTS:-}" diff --git a/google/cloud/future_generic.h b/google/cloud/future_generic.h index ddfc718..24ae7a4 100644 --- a/google/cloud/future_generic.h +++ b/google/cloud/future_generic.h @@ -90,14 +90,14 @@ class future final : private internal::future_base { * ready. The return type is a future wrapping the return type of * @a func. * - * @return future where T is std::result_of_t (basically). - * If T matches future then it returns future. The returned + * @return `future` where T is `std::result_of_t` (basically). + * If T matches `future` then it returns `future`. The returned * future will contain the result of @a func. * @param func a Callable to be invoked when the future is ready. * The function might be called immediately, e.g., if the future is * ready. * - * Side effects: valid() == false if the operation is successful. + * Side effects: `valid() == false` if the operation is successful. */ template typename internal::then_helper::future_t then(F&& func) { diff --git a/google/cloud/future_generic_test.cc b/google/cloud/future_generic_test.cc index cb2a0a9..bbbb744 100644 --- a/google/cloud/future_generic_test.cc +++ b/google/cloud/future_generic_test.cc @@ -98,11 +98,12 @@ TEST(FutureTestInt, conform_30_6_5_7) { EXPECT_TRUE(f0.valid()); ASSERT_EQ(std::future_status::ready, f0.wait_for(0_ms)); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { f0.get(); } catch (std::future_error const& ex) { - EXPECT_EQ(std::future_errc::broken_promise, ex.code()); - throw; - }, - std::future_error); + EXPECT_THROW( + try { f0.get(); } catch (std::future_error const& ex) { + EXPECT_EQ(std::future_errc::broken_promise, ex.code()); + throw; + }, + std::future_error); #else EXPECT_DEATH_IF_SUPPORTED( f0.get(), @@ -201,11 +202,12 @@ TEST(FutureTestInt, conform_30_6_5_18) { p0.set_exception(std::make_exception_ptr(std::runtime_error("testing"))); ASSERT_EQ(std::future_status::ready, f0.wait_for(0_ms)); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { f0.get(); } catch (std::runtime_error const& ex) { - EXPECT_EQ(std::string("testing"), ex.what()); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { f0.get(); } catch (std::runtime_error const& ex) { + EXPECT_EQ(std::string("testing"), ex.what()); + throw; + }, + std::runtime_error); #else EXPECT_DEATH_IF_SUPPORTED( f0.get(), @@ -430,11 +432,12 @@ TEST(FutureTestInt, conform_30_6_6_17) { future f = p.get_future(); p.set_exception(std::make_exception_ptr(std::runtime_error("test message"))); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { f.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { f.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else EXPECT_DEATH_IF_SUPPORTED( f.get(), diff --git a/google/cloud/future_generic_then_test.cc b/google/cloud/future_generic_then_test.cc index 785baf6..c07ed2b 100644 --- a/google/cloud/future_generic_then_test.cc +++ b/google/cloud/future_generic_then_test.cc @@ -74,11 +74,12 @@ TEST(FutureTestInt, ThenException) { EXPECT_TRUE(next.valid()); EXPECT_EQ(std::future_status::ready, next.wait_for(0_ms)); - EXPECT_THROW(try { next.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { next.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); EXPECT_FALSE(next.valid()); #else EXPECT_DEATH_IF_SUPPORTED(p.set_value(42), "test message"); @@ -233,11 +234,12 @@ TEST(FutureTestInt, conform_2_3_3_b) { #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS p.set_exception(std::make_exception_ptr(std::runtime_error("test message"))); EXPECT_TRUE(unwrapped.is_ready()); - EXPECT_THROW(try { unwrapped.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { unwrapped.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else EXPECT_DEATH_IF_SUPPORTED( p.set_exception( @@ -263,11 +265,12 @@ TEST(FutureTestInt, conform_2_3_3_c) { #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS p2.set_exception(std::make_exception_ptr(std::runtime_error("test message"))); EXPECT_TRUE(unwrapped.is_ready()); - EXPECT_THROW(try { unwrapped.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { unwrapped.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else EXPECT_DEATH_IF_SUPPORTED( p2.set_exception( @@ -290,11 +293,12 @@ TEST(FutureTestInt, conform_2_3_3_d) { p.set_value(future{}); EXPECT_TRUE(unwrapped.is_ready()); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { unwrapped.get(); } catch (std::future_error const& ex) { - EXPECT_EQ(std::future_errc::broken_promise, ex.code()); - throw; - }, - std::future_error); + EXPECT_THROW( + try { unwrapped.get(); } catch (std::future_error const& ex) { + EXPECT_EQ(std::future_errc::broken_promise, ex.code()); + throw; + }, + std::future_error); #else EXPECT_DEATH_IF_SUPPORTED( unwrapped.get(), @@ -429,11 +433,12 @@ TEST(FutureTestInt, conform_2_3_8_e) { #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS p.set_value(42); EXPECT_EQ(std::future_status::ready, next.wait_for(0_ms)); - EXPECT_THROW(try { next.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test exception in functor")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { next.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test exception in functor")); + throw; + }, + std::runtime_error); EXPECT_FALSE(next.valid()); #else EXPECT_DEATH_IF_SUPPORTED(p.set_value(42), "test exception in functor"); @@ -537,11 +542,12 @@ TEST(FutureTestInt, conform_2_3_9_d) { p.set_exception(std::make_exception_ptr(std::runtime_error("test message"))); EXPECT_TRUE(called); EXPECT_TRUE(r.is_ready()); - EXPECT_THROW(try { r.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { r.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else // With exceptions disabled the program terminates as soon as the exception is // set. @@ -574,11 +580,12 @@ TEST(FutureTestInt, conform_2_3_9_e) { EXPECT_TRUE(r.is_ready()); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { r.get(); } catch (std::future_error const& ex) { - EXPECT_EQ(std::future_errc::broken_promise, ex.code()); - throw; - }, - std::future_error); + EXPECT_THROW( + try { r.get(); } catch (std::future_error const& ex) { + EXPECT_EQ(std::future_errc::broken_promise, ex.code()); + throw; + }, + std::future_error); #else EXPECT_DEATH_IF_SUPPORTED( r.get(), diff --git a/google/cloud/future_void.h b/google/cloud/future_void.h index f6d1c8b..eb1e794 100644 --- a/google/cloud/future_void.h +++ b/google/cloud/future_void.h @@ -87,14 +87,14 @@ class future final : private internal::future_base { * ready. The return type is a future wrapping the return type of * @a func. * - * @return future where T is std::result_of_t (basically). - * If T matches future then it returns future. The returned + * @return `future` where T is `std::result_of_t` (basically). + * If T matches `future` then it returns `future`. The returned * future will contain the result of @a func. * @param func a Callable to be invoked when the future is ready. * The function might be called immediately, e.g., if the future is * ready. * - * Side effects: valid() == false if the operation is successful. + * Side effects: `valid() == false` if the operation is successful. */ template typename internal::then_helper::future_t then(F&& func) { diff --git a/google/cloud/future_void_test.cc b/google/cloud/future_void_test.cc index 02478c5..ed450d0 100644 --- a/google/cloud/future_void_test.cc +++ b/google/cloud/future_void_test.cc @@ -105,11 +105,12 @@ TEST(FutureTestVoid, conform_30_6_5_7) { EXPECT_TRUE(f0.valid()); ASSERT_EQ(std::future_status::ready, f0.wait_for(0_ms)); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { f0.get(); } catch (std::future_error const& ex) { - EXPECT_EQ(std::future_errc::broken_promise, ex.code()); - throw; - }, - std::future_error); + EXPECT_THROW( + try { f0.get(); } catch (std::future_error const& ex) { + EXPECT_EQ(std::future_errc::broken_promise, ex.code()); + throw; + }, + std::future_error); #else EXPECT_DEATH_IF_SUPPORTED( f0.get(), @@ -208,11 +209,12 @@ TEST(FutureTestVoid, conform_30_6_5_18) { p0.set_exception(std::make_exception_ptr(std::runtime_error("testing"))); ASSERT_EQ(std::future_status::ready, f0.wait_for(0_ms)); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { f0.get(); } catch (std::runtime_error const& ex) { - EXPECT_EQ(std::string("testing"), ex.what()); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { f0.get(); } catch (std::runtime_error const& ex) { + EXPECT_EQ(std::string("testing"), ex.what()); + throw; + }, + std::runtime_error); #else EXPECT_DEATH_IF_SUPPORTED( f0.get(), @@ -437,11 +439,12 @@ TEST(FutureTestVoid, conform_30_6_6_17) { future f = p.get_future(); p.set_exception(std::make_exception_ptr(std::runtime_error("test message"))); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { f.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { f.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else EXPECT_DEATH_IF_SUPPORTED( f.get(), diff --git a/google/cloud/future_void_then_test.cc b/google/cloud/future_void_then_test.cc index 16ff0e8..aaad528 100644 --- a/google/cloud/future_void_then_test.cc +++ b/google/cloud/future_void_then_test.cc @@ -68,11 +68,12 @@ TEST(FutureTestVoid, ThenException) { EXPECT_TRUE(next.valid()); EXPECT_EQ(std::future_status::ready, next.wait_for(0_ms)); - EXPECT_THROW(try { next.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { next.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); EXPECT_FALSE(next.valid()); #else EXPECT_DEATH_IF_SUPPORTED(p.set_value(), "test message"); @@ -220,11 +221,12 @@ TEST(FutureTestVoid, conform_2_3_3_b) { #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS p.set_exception(std::make_exception_ptr(std::runtime_error("test message"))); EXPECT_TRUE(unwrapped.is_ready()); - EXPECT_THROW(try { unwrapped.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { unwrapped.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else EXPECT_DEATH_IF_SUPPORTED( p.set_exception( @@ -250,11 +252,12 @@ TEST(FutureTestVoid, conform_2_3_3_c) { #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS p2.set_exception(std::make_exception_ptr(std::runtime_error("test message"))); EXPECT_TRUE(unwrapped.is_ready()); - EXPECT_THROW(try { unwrapped.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { unwrapped.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else std::string expected = "future_error\\["; expected += std::make_error_code(std::future_errc::promise_already_satisfied) @@ -281,11 +284,12 @@ TEST(FutureTestVoid, conform_2_3_3_d) { EXPECT_TRUE(unwrapped.is_ready()); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { unwrapped.get(); } catch (std::future_error const& ex) { - EXPECT_EQ(std::future_errc::broken_promise, ex.code()); - throw; - }, - std::future_error); + EXPECT_THROW( + try { unwrapped.get(); } catch (std::future_error const& ex) { + EXPECT_EQ(std::future_errc::broken_promise, ex.code()); + throw; + }, + std::future_error); #else EXPECT_DEATH_IF_SUPPORTED( unwrapped.get(), @@ -420,11 +424,12 @@ TEST(FutureTestVoid, conform_2_3_8_e) { #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS p.set_value(); EXPECT_EQ(std::future_status::ready, next.wait_for(0_ms)); - EXPECT_THROW(try { next.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test exception in functor")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { next.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test exception in functor")); + throw; + }, + std::runtime_error); EXPECT_FALSE(next.valid()); #else EXPECT_DEATH_IF_SUPPORTED(p.set_value(), "test exception in functor"); @@ -529,11 +534,12 @@ TEST(FutureTestVoid, conform_2_3_9_d) { EXPECT_TRUE(called); EXPECT_TRUE(r.is_ready()); - EXPECT_THROW(try { r.get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { r.get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else // With exceptions disabled the program terminates as soon as the exception is // set. @@ -565,11 +571,12 @@ TEST(FutureTestVoid, conform_2_3_9_e) { EXPECT_TRUE(called); EXPECT_TRUE(r.is_ready()); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { r.get(); } catch (std::future_error const& ex) { - EXPECT_EQ(std::future_errc::broken_promise, ex.code()); - throw; - }, - std::future_error); + EXPECT_THROW( + try { r.get(); } catch (std::future_error const& ex) { + EXPECT_EQ(std::future_errc::broken_promise, ex.code()); + throw; + }, + std::future_error); #else // With exceptions disabled setting the value immediately terminates. EXPECT_DEATH_IF_SUPPORTED( diff --git a/google/cloud/internal/build_info.cc.in b/google/cloud/internal/build_info.cc.in index 0a356ec..0e2563d 100644 --- a/google/cloud/internal/build_info.cc.in +++ b/google/cloud/internal/build_info.cc.in @@ -29,6 +29,7 @@ std::string compiler() { // NOLINTNEXTLINE(readability-identifier-naming) std::string compiler_flags() { + // NOLINTNEXTLINE(modernize-avoid-c-arrays) static char const kCompilerFlags[] = R"""(@CMAKE_CXX_FLAGS@ ${CMAKE_CXX_FLAGS_${GOOGLE_CLOUD_CPP_BUILD_TYPE_UPPER}})"""; return kCompilerFlags; @@ -38,7 +39,7 @@ std::string compiler_flags() { std::string build_metadata() { // Sometimes GOOGLE_CLOUD_CPP_BUILD_METADATA string expands to nothing, and // then clang-tidy complains. - // NOLINTNEXTLINE(readability-redundant-string-init) + // NOLINTNEXTLINE(readability-redundant-string-init,modernize-avoid-c-arrays) static char const kBuildMetadata[] = R"""(@GOOGLE_CLOUD_CPP_BUILD_METADATA@)"""; return kBuildMetadata; } diff --git a/google/cloud/internal/compiler_info.cc b/google/cloud/internal/compiler_info.cc index 551bb8f..7792d2b 100644 --- a/google/cloud/internal/compiler_info.cc +++ b/google/cloud/internal/compiler_info.cc @@ -81,14 +81,18 @@ std::string CompilerFeatures() { } std::string LanguageVersion() { + constexpr auto kMagicVersionCxx98 = 199711L; + constexpr auto kMagicVersionCxx11 = 201103L; + constexpr auto kMagicVersionCxx14 = 201402L; + constexpr auto kMagicVersionCxx17 = 201703L; switch (__cplusplus) { - case 199711L: + case kMagicVersionCxx98: return "1998"; - case 201103L: + case kMagicVersionCxx11: return "2011"; - case 201402L: + case kMagicVersionCxx14: return "2014"; - case 201703L: + case kMagicVersionCxx17: return "2017"; default: return "unknown"; diff --git a/google/cloud/internal/filesystem_test.cc b/google/cloud/internal/filesystem_test.cc index 429a500..ba5a6df 100644 --- a/google/cloud/internal/filesystem_test.cc +++ b/google/cloud/internal/filesystem_test.cc @@ -253,12 +253,14 @@ TEST(FilesystemTest, StatusErrorDoesThrow) { std::ofstream(file_name).close(); auto path = file_name + "/files/cannot/be/directories"; #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { status(path); } catch (std::system_error const& ex) { - EXPECT_EQ(static_cast(std::errc::not_a_directory), ex.code().value()); - EXPECT_THAT(ex.what(), HasSubstr(path)); - throw; - }, - std::system_error); + EXPECT_THROW( + try { status(path); } catch (std::system_error const& ex) { + EXPECT_EQ(static_cast(std::errc::not_a_directory), + ex.code().value()); + EXPECT_THAT(ex.what(), HasSubstr(path)); + throw; + }, + std::system_error); #else EXPECT_DEATH_IF_SUPPORTED(status(path), "exceptions are disabled"); #endif // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS @@ -303,13 +305,14 @@ TEST(FilesystemTest, FileSizeNotFound) { TEST(FilesystemTest, FileSizeNotFoundDoesThrow) { auto path = CreateRandomFileName(); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { file_size(path); } catch (std::system_error const& ex) { - EXPECT_EQ(static_cast(std::errc::no_such_file_or_directory), - ex.code().value()); - EXPECT_THAT(ex.what(), HasSubstr(path)); - throw; - }, - std::system_error); + EXPECT_THROW( + try { file_size(path); } catch (std::system_error const& ex) { + EXPECT_EQ(static_cast(std::errc::no_such_file_or_directory), + ex.code().value()); + EXPECT_THAT(ex.what(), HasSubstr(path)); + throw; + }, + std::system_error); #else EXPECT_DEATH_IF_SUPPORTED(file_size(path), "exceptions are disabled"); #endif // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS diff --git a/google/cloud/internal/format_time_point.cc b/google/cloud/internal/format_time_point.cc index 921c6db..e6f8f4a 100644 --- a/google/cloud/internal/format_time_point.cc +++ b/google/cloud/internal/format_time_point.cc @@ -14,6 +14,7 @@ #include "google/cloud/internal/format_time_point.h" #include "google/cloud/internal/throw_delegate.h" +#include #include #include #include @@ -26,23 +27,31 @@ std::string FormatFractional(std::chrono::nanoseconds ns) { return ""; } - char buffer[16]; + using std::chrono::milliseconds; + using std::chrono::nanoseconds; + using std::chrono::seconds; + constexpr int kMaxNanosecondsDigits = 9; + constexpr int kNanosecondsPerMillisecond = + nanoseconds(milliseconds(1)).count(); + constexpr int kMillisecondsPerSecond = milliseconds(seconds(1)).count(); + // Need to leave room for the period and the NUL terminator + std::array buffer{}; // If the fractional seconds can be just expressed as milliseconds, do that, // we do not want to print 1.123000000 - auto d = std::lldiv(ns.count(), 1000000LL); + auto d = std::lldiv(ns.count(), kNanosecondsPerMillisecond); if (d.rem == 0) { - std::snprintf(buffer, sizeof(buffer), ".%03lld", d.quot); - return buffer; + std::snprintf(buffer.data(), buffer.size(), ".%03lld", d.quot); + return buffer.data(); } - d = std::lldiv(ns.count(), 1000LL); + d = std::lldiv(ns.count(), kMillisecondsPerSecond); if (d.rem == 0) { - std::snprintf(buffer, sizeof(buffer), ".%06lld", d.quot); - return buffer; + std::snprintf(buffer.data(), buffer.size(), ".%06lld", d.quot); + return buffer.data(); } - std::snprintf(buffer, sizeof(buffer), ".%09lld", + std::snprintf(buffer.data(), buffer.size(), ".%09lld", static_cast(ns.count())); - return buffer; + return buffer.data(); } std::tm AsUtcTm(std::chrono::system_clock::time_point tp) { @@ -64,12 +73,24 @@ namespace cloud { inline namespace GOOGLE_CLOUD_CPP_NS { namespace internal { +constexpr int kTimestampFormatSize = 256; +static_assert(kTimestampFormatSize > ((4 + 1) // YYYY- + + (2 + 1) // MM- + + 2 // DD + + 1 // T + + (2 + 1) // HH: + + (2 + 1) // MM: + + 2 // SS + + 1) // Z + , + "Buffer size not large enough for YYYY-MM-DDTHH:MM:SSZ format"); + std::string FormatRfc3339(std::chrono::system_clock::time_point tp) { std::tm tm = AsUtcTm(tp); - char buffer[256]; - std::strftime(buffer, sizeof(buffer), "%Y-%m-%dT%H:%M:%S", &tm); + std::array buffer{}; + std::strftime(buffer.data(), buffer.size(), "%Y-%m-%dT%H:%M:%S", &tm); - std::string result(buffer); + std::string result(buffer.data()); // Add the fractional seconds... auto duration = tp.time_since_epoch(); using std::chrono::duration_cast; @@ -83,16 +104,23 @@ std::string FormatRfc3339(std::chrono::system_clock::time_point tp) { std::string FormatV4SignedUrlTimestamp( std::chrono::system_clock::time_point tp) { std::tm tm = AsUtcTm(tp); - char buffer[256]; - std::strftime(buffer, sizeof(buffer), "%Y%m%dT%H%M%SZ", &tm); - return buffer; + std::array buffer{}; + std::strftime(buffer.data(), buffer.size(), "%Y%m%dT%H%M%SZ", &tm); + return buffer.data(); } std::string FormatV4SignedUrlScope(std::chrono::system_clock::time_point tp) { std::tm tm = AsUtcTm(tp); - char buffer[256]; - std::strftime(buffer, sizeof(buffer), "%Y%m%d", &tm); - return buffer; + + constexpr int kDateFormatSize = 256; + static_assert(kDateFormatSize > (4 // YYYY + + 2 // MM + + 2) // DD + , + "Buffer size not large enough for YYYYMMDD format"); + std::array buffer{}; + std::strftime(buffer.data(), buffer.size(), "%Y%m%d", &tm); + return buffer.data(); } } // namespace internal diff --git a/google/cloud/internal/future_impl.h b/google/cloud/internal/future_impl.h index 75a703d..d51d571 100644 --- a/google/cloud/internal/future_impl.h +++ b/google/cloud/internal/future_impl.h @@ -56,15 +56,15 @@ class continuation_base { /** * Common base class for all shared state classes. * - * The implementation of the shared state for future, future and - * future share a lot of code. This class refactors that code, it + * The implementation of the shared state for `future`, `future` and + * `future` share a lot of code. This class refactors that code, it * represents a shared state of unknown type. * * @note While most of the invariants for promises and futures are implemented * by this class, not all of them are. Notably, future values can only be - * retrieved once, but this is enforced because calling .get() or .then() on a - * future invalidates the future for further use. The shared state does not - * record that state change. + * retrieved once, but this is enforced because calling `.get()` or `.then()` + * on a future invalidates the future for further use. The shared state does + * not record that state change. */ class future_shared_state_base { public: diff --git a/google/cloud/internal/future_impl_test.cc b/google/cloud/internal/future_impl_test.cc index 9cf1611..9caf28d 100644 --- a/google/cloud/internal/future_impl_test.cc +++ b/google/cloud/internal/future_impl_test.cc @@ -139,11 +139,12 @@ TEST(ContinuationVoidTest, SetExceptionCallsContinuation) { std::make_exception_ptr(std::runtime_error("test message"))); EXPECT_TRUE(called); EXPECT_TRUE(output->is_ready()); - EXPECT_THROW(try { output->get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { output->get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else EXPECT_DEATH_IF_SUPPORTED( input->set_exception( @@ -226,11 +227,12 @@ TEST(FutureImplVoid, Abandon) { shared_state.abandon(); EXPECT_TRUE(shared_state.is_ready()); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { shared_state.get(); } catch (std::future_error const& ex) { - EXPECT_EQ(std::future_errc::broken_promise, ex.code()); - throw; - }, - std::future_error); + EXPECT_THROW( + try { shared_state.get(); } catch (std::future_error const& ex) { + EXPECT_EQ(std::future_errc::broken_promise, ex.code()); + throw; + }, + std::future_error); #else EXPECT_DEATH_IF_SUPPORTED( shared_state.get(), @@ -371,11 +373,12 @@ TEST(FutureImplInt, Abandon) { shared_state.abandon(); EXPECT_TRUE(shared_state.is_ready()); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { shared_state.get(); } catch (std::future_error const& ex) { - EXPECT_EQ(std::future_errc::broken_promise, ex.code()); - throw; - }, - std::future_error); + EXPECT_THROW( + try { shared_state.get(); } catch (std::future_error const& ex) { + EXPECT_EQ(std::future_errc::broken_promise, ex.code()); + throw; + }, + std::future_error); #else EXPECT_DEATH_IF_SUPPORTED( shared_state.get(), @@ -479,11 +482,12 @@ TEST(ContinuationIntTest, SetExceptionCallsContinuation) { std::make_exception_ptr(std::runtime_error("test message"))); EXPECT_TRUE(called); EXPECT_TRUE(output->is_ready()); - EXPECT_THROW(try { output->get(); } catch (std::runtime_error const& ex) { - EXPECT_THAT(ex.what(), HasSubstr("test message")); - throw; - }, - std::runtime_error); + EXPECT_THROW( + try { output->get(); } catch (std::runtime_error const& ex) { + EXPECT_THAT(ex.what(), HasSubstr("test message")); + throw; + }, + std::runtime_error); #else EXPECT_DEATH_IF_SUPPORTED( input->set_exception( diff --git a/google/cloud/internal/future_then_meta.h b/google/cloud/internal/future_then_meta.h index 452f870..1bfa6f6 100644 --- a/google/cloud/internal/future_then_meta.h +++ b/google/cloud/internal/future_then_meta.h @@ -32,21 +32,21 @@ namespace internal { template class future_shared_state; -/// Compute the return type for a future::then() +/// Compute the return type for a `future::then()` template struct unwrap_then { using type = FunctorReturn; using requires_unwrap_t = std::false_type; }; -/// Specialize the unwrap_then<> for functors that return future +/// Specialize the `unwrap_then<>` for functors that return `future` template struct unwrap_then> { using type = U; using requires_unwrap_t = std::true_type; }; -/// Specialize the unwrap_then<> for functors that return future +/// Specialize the `unwrap_then<>` for functors that return `future` template struct unwrap_internal { using type = U; diff --git a/google/cloud/internal/pagination_range_test.cc b/google/cloud/internal/pagination_range_test.cc index fe90e37..03928e9 100644 --- a/google/cloud/internal/pagination_range_test.cc +++ b/google/cloud/internal/pagination_range_test.cc @@ -51,8 +51,8 @@ TEST(RangeFromPagination, Empty) { return response; })); - TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, - GetItems); + TestedRange range( + Request{}, [&](Request const& r) { return mock.Loader(r); }, GetItems); EXPECT_TRUE(range.begin() == range.end()); } @@ -67,8 +67,8 @@ TEST(RangeFromPagination, SinglePage) { return response; })); - TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, - GetItems); + TestedRange range( + Request{}, [&](Request const& r) { return mock.Loader(r); }, GetItems); std::vector names; for (auto& p : range) { if (!p) break; @@ -97,8 +97,8 @@ TEST(RangeFromPagination, TwoPages) { return response; })); - TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, - GetItems); + TestedRange range( + Request{}, [&](Request const& r) { return mock.Loader(r); }, GetItems); std::vector names; for (auto& p : range) { if (!p) break; @@ -131,8 +131,8 @@ TEST(RangeFromPagination, TwoPagesWithError) { return Status(StatusCode::kAborted, "bad-luck"); })); - TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, - GetItems); + TestedRange range( + Request{}, [&](Request const& r) { return mock.Loader(r); }, GetItems); std::vector names; for (auto& p : range) { if (!p) { @@ -160,8 +160,8 @@ TEST(RangeFromPagination, IteratorCoverage) { return Status(StatusCode::kAborted, "bad-luck"); })); - TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, - GetItems); + TestedRange range( + Request{}, [&](Request const& r) { return mock.Loader(r); }, GetItems); auto i0 = range.begin(); auto i1 = i0; EXPECT_TRUE(i0 == i1); diff --git a/google/cloud/internal/parse_rfc3339.cc b/google/cloud/internal/parse_rfc3339.cc index 29474fb..856468f 100644 --- a/google/cloud/internal/parse_rfc3339.cc +++ b/google/cloud/internal/parse_rfc3339.cc @@ -30,17 +30,25 @@ namespace { } bool IsLeapYear(int year) { + // NOLINTNEXTLINE(readability-magic-numbers) return (year % 4 == 0 && (year % 100 != 0 || year % 400 == 0)); } +constexpr int kMonthsInYear = 12; +constexpr int kHoursInDay = 24; +constexpr int kMinutesInHour = + std::chrono::seconds(std::chrono::minutes(1)).count(); +constexpr int kSecondsInMinute = + std::chrono::minutes(std::chrono::hours(1)).count(); + std::chrono::system_clock::time_point ParseDateTime( char const*& buffer, std::string const& timestamp) { // Use std::mktime to compute the number of seconds because RFC 3339 requires // working with civil time, including the annoying leap seconds, and mktime // does. - int year, month, day; + int year, month, day; // NOLINT(readability-isolate-declaration) char date_time_separator; - int hours, minutes, seconds; + int hours, minutes, seconds; // NOLINT(readability-isolate-declaration) int pos; auto count = @@ -57,10 +65,8 @@ std::chrono::system_clock::time_point ParseDateTime( if (date_time_separator != 'T' && date_time_separator != 't') { ReportError(timestamp, "Invalid date-time separator, expected 'T' or 't'."); } - if (month < 1 || month > 12) { - ReportError(timestamp, "Out of range month."); - } - constexpr int kMaxDaysInMonth[] = { + + constexpr std::array kMaxDaysInMonth{ 31, // January 29, // February (non-leap years checked below) 31, // March @@ -74,16 +80,20 @@ std::chrono::system_clock::time_point ParseDateTime( 30, // November 31, // December }; + constexpr int kMkTimeBaseYear = 1900; + if (month < 1 || month > kMonthsInYear) { + ReportError(timestamp, "Out of range month."); + } if (day < 1 || day > kMaxDaysInMonth[month - 1]) { ReportError(timestamp, "Out of range day for given month."); } - if (2 == month && day > 28 && !IsLeapYear(year)) { + if (2 == month && day > kMaxDaysInMonth[1] - 1 && !IsLeapYear(year)) { ReportError(timestamp, "Out of range day for given month."); } - if (hours < 0 || hours > 23) { + if (hours < 0 || hours >= kHoursInDay) { ReportError(timestamp, "Out of range hour."); } - if (minutes < 0 || minutes > 59) { + if (minutes < 0 || minutes >= kMinutesInHour - 1) { ReportError(timestamp, "Out of range minute."); } // RFC-3339 points out that the seconds field can only assume value '60' for @@ -91,14 +101,14 @@ std::chrono::system_clock::time_point ParseDateTime( // should valid that `seconds` is smaller than 59 for negative leap seconds). // This would require loading a table, and adds too much complexity for little // value. - if (seconds < 0 || seconds > 60) { + if (seconds < 0 || seconds > kSecondsInMinute) { ReportError(timestamp, "Out of range second."); } // Advance the pointer for all the characters read. buffer += pos; std::tm tm{}; - tm.tm_year = year - 1900; + tm.tm_year = year - kMkTimeBaseYear; tm.tm_mon = month - 1; tm.tm_mday = day; tm.tm_hour = hours; @@ -114,15 +124,17 @@ std::chrono::system_clock::duration ParseFractionalSeconds( } ++buffer; - long fractional_seconds; + long fractional_seconds; // NOLINT(google-runtime-int) int pos; auto count = std::sscanf(buffer, "%9ld%n", &fractional_seconds, &pos); if (count != 1) { ReportError(timestamp, "Invalid fractional seconds component."); } + constexpr int kMaxNanosecondDigits = 9; + constexpr int kNanosecondsBase = 10; // Normalize the fractional seconds to nanoseconds. - for (int digits = pos; digits < 9; ++digits) { - fractional_seconds *= 10; + for (int digits = pos; digits < kMaxNanosecondDigits; ++digits) { + fractional_seconds *= kNanosecondsBase; } // Skip any other digits. This loses precision for sub-nanosecond timestamps, // but we do not consider this a problem for Internet timestamps. @@ -140,17 +152,17 @@ std::chrono::seconds ParseOffset(char const*& buffer, bool positive = (buffer[0] == '+'); ++buffer; // Parse the HH:MM offset. - int hours, minutes, pos; + int hours, minutes, pos; // NOLINT(readability-isolate-declaration) auto count = std::sscanf(buffer, "%2d:%2d%n", &hours, &minutes, &pos); constexpr int kExpectedOffsetWidth = 5; constexpr int kExpectedOffsetFields = 2; if (count != kExpectedOffsetFields || pos != kExpectedOffsetWidth) { ReportError(timestamp, "Invalid timezone offset, expected [+-]HH:MM."); } - if (hours < 0 || hours > 23) { + if (hours < 0 || hours >= kHoursInDay) { ReportError(timestamp, "Out of range offset hour."); } - if (minutes < 0 || minutes > 59) { + if (minutes < 0 || minutes >= kMinutesInHour) { ReportError(timestamp, "Out of range offset minute."); } buffer += pos; diff --git a/google/cloud/log.cc b/google/cloud/log.cc index f4a034a..741c111 100644 --- a/google/cloud/log.cc +++ b/google/cloud/log.cc @@ -26,7 +26,9 @@ static_assert(static_cast(Severity::GCP_LS_LOWEST) < "Expect LOWEST severity to be smaller than HIGHEST severity"); std::ostream& operator<<(std::ostream& os, Severity x) { - char const* names[] = { + auto constexpr kSeverityCount = + static_cast(Severity::GCP_LS_HIGHEST) + 1; + std::array names{ "TRACE", "DEBUG", "INFO", "NOTICE", "WARNING", "ERROR", "CRITICAL", "ALERT", "FATAL", }; @@ -56,11 +58,13 @@ LogSink& LogSink::Instance() { return *kInstance; } +// NOLINTNEXTLINE(google-runtime-int) long LogSink::AddBackend(std::shared_ptr backend) { std::unique_lock lk(mu_); return AddBackendImpl(std::move(backend)); } +// NOLINTNEXTLINE(google-runtime-int) void LogSink::RemoveBackend(long id) { std::unique_lock lk(mu_); RemoveBackendImpl(id); @@ -134,13 +138,15 @@ void LogSink::DisableStdClogImpl() { clog_backend_id_ = 0; } +// NOLINTNEXTLINE(google-runtime-int) long LogSink::AddBackendImpl(std::shared_ptr backend) { - long id = ++next_id_; + auto const id = ++next_id_; backends_.emplace(id, std::move(backend)); empty_.store(backends_.empty()); return id; } +// NOLINTNEXTLINE(google-runtime-int) void LogSink::RemoveBackendImpl(long id) { auto it = backends_.find(id); if (backends_.end() == it) { diff --git a/google/cloud/optional_test.cc b/google/cloud/optional_test.cc index a198c12..aa051c9 100644 --- a/google/cloud/optional_test.cc +++ b/google/cloud/optional_test.cc @@ -19,7 +19,7 @@ namespace google { namespace cloud { inline namespace GOOGLE_CLOUD_CPP_NS { -/// Helper types to test google::cloud::optional +/// Helper types to test `google::cloud::optional` namespace { using testing_util::Observable; diff --git a/google/cloud/testing_util/expect_exception.h b/google/cloud/testing_util/expect_exception.h index 3654397..dc19578 100644 --- a/google/cloud/testing_util/expect_exception.h +++ b/google/cloud/testing_util/expect_exception.h @@ -75,11 +75,12 @@ void ExpectException( std::function const& validator, char const* expected_message) { #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { expression(); } catch (ExpectedException const& ex) { - validator(ex); - throw; - }, - ExpectedException); + EXPECT_THROW( + try { expression(); } catch (ExpectedException const& ex) { + validator(ex); + throw; + }, + ExpectedException); (void)expected_message; // suppress clang-tidy warning. #else (void)validator; // suppress clang-tidy warning. diff --git a/google/cloud/testing_util/expect_future_error.h b/google/cloud/testing_util/expect_future_error.h index 27dced5..6a9597b 100644 --- a/google/cloud/testing_util/expect_future_error.h +++ b/google/cloud/testing_util/expect_future_error.h @@ -35,11 +35,12 @@ namespace testing_util { template void ExpectFutureError(Functor functor, std::future_errc code) { #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS - EXPECT_THROW(try { functor(); } catch (std::future_error const& ex) { - EXPECT_EQ(code, ex.code()); - throw; - }, - std::future_error); + EXPECT_THROW( + try { functor(); } catch (std::future_error const& ex) { + EXPECT_EQ(code, ex.code()); + throw; + }, + std::future_error); #else std::string expected = "future_error\\["; expected += std::make_error_code(code).message(); From 4ed33eac9090bdcb4f0b2da4e3931df14864415b Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 6 Mar 2020 09:06:01 -0500 Subject: [PATCH 2/8] Link bug in TODO --- .clang-tidy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 561222c..6f486ae 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -48,7 +48,7 @@ Checks: > # Turn all the warnings from the checks above into errors. WarningsAsErrors: "*" -# TODO(#...) - Enable clang-tidy checks in our headers. +# TODO(#205) - Enable clang-tidy checks in our headers. # HeaderFilterRegex: "google/cloud/.*" CheckOptions: From 1049dfca1baf645f853103cbb2fb59c513ff2050 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 6 Mar 2020 09:06:38 -0500 Subject: [PATCH 3/8] Add missing includes for libc++ --- google/cloud/internal/parse_rfc3339.cc | 1 + google/cloud/log.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/google/cloud/internal/parse_rfc3339.cc b/google/cloud/internal/parse_rfc3339.cc index 856468f..1cd8097 100644 --- a/google/cloud/internal/parse_rfc3339.cc +++ b/google/cloud/internal/parse_rfc3339.cc @@ -14,6 +14,7 @@ #include "google/cloud/internal/parse_rfc3339.h" #include "google/cloud/internal/throw_delegate.h" +#include #include #include #include diff --git a/google/cloud/log.cc b/google/cloud/log.cc index 741c111..fbb76d9 100644 --- a/google/cloud/log.cc +++ b/google/cloud/log.cc @@ -14,6 +14,7 @@ #include "google/cloud/log.h" #include "google/cloud/internal/getenv.h" +#include namespace google { namespace cloud { From 21b260b7db14ab1a5b8cdc98d8f321d5397d0f28 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 6 Mar 2020 09:07:22 -0500 Subject: [PATCH 4/8] Fix false-positive warnings, the buffer was large enough, but whatevs. --- google/cloud/internal/format_time_point.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/google/cloud/internal/format_time_point.cc b/google/cloud/internal/format_time_point.cc index e6f8f4a..7b7824f 100644 --- a/google/cloud/internal/format_time_point.cc +++ b/google/cloud/internal/format_time_point.cc @@ -31,11 +31,17 @@ std::string FormatFractional(std::chrono::nanoseconds ns) { using std::chrono::nanoseconds; using std::chrono::seconds; constexpr int kMaxNanosecondsDigits = 9; + auto constexpr kBufferSize = 16; + static_assert(kBufferSize > (kMaxNanosecondsDigits // digits + + 1 // period + + 1) // NUL terminator + , + "Buffer is not large enough for printing nanoseconds"); constexpr int kNanosecondsPerMillisecond = nanoseconds(milliseconds(1)).count(); constexpr int kMillisecondsPerSecond = milliseconds(seconds(1)).count(); - // Need to leave room for the period and the NUL terminator - std::array buffer{}; + + std::array buffer{}; // If the fractional seconds can be just expressed as milliseconds, do that, // we do not want to print 1.123000000 auto d = std::lldiv(ns.count(), kNanosecondsPerMillisecond); From b318793eb719c9b414cc591583be16c348dd346f Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 6 Mar 2020 09:15:35 -0500 Subject: [PATCH 5/8] Disable formatting for no-tests build. --- ci/kokoro/docker/build.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/ci/kokoro/docker/build.sh b/ci/kokoro/docker/build.sh index 471a4c5..9b305d9 100755 --- a/ci/kokoro/docker/build.sh +++ b/ci/kokoro/docker/build.sh @@ -136,7 +136,6 @@ elif [[ "${BUILD_NAME}" = "no-tests" ]]; then # package maintainers, where the cost of running the tests for a fixed version # is too high. export BUILD_TESTING=no - export CHECK_STYLE=yes elif [[ "${BUILD_NAME}" = "libcxx" ]]; then # Compile using libc++. This is easier to install on Fedora. export CC=clang From 2c27c9326356481510888f9a4440b96def80aabb Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 6 Mar 2020 09:23:22 -0500 Subject: [PATCH 6/8] Fix formatting. --- google/cloud/internal/format_time_point.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google/cloud/internal/format_time_point.cc b/google/cloud/internal/format_time_point.cc index 7b7824f..c319771 100644 --- a/google/cloud/internal/format_time_point.cc +++ b/google/cloud/internal/format_time_point.cc @@ -33,9 +33,9 @@ std::string FormatFractional(std::chrono::nanoseconds ns) { constexpr int kMaxNanosecondsDigits = 9; auto constexpr kBufferSize = 16; static_assert(kBufferSize > (kMaxNanosecondsDigits // digits - + 1 // period - + 1) // NUL terminator - , + + 1 // period + + 1) // NUL terminator + , "Buffer is not large enough for printing nanoseconds"); constexpr int kNanosecondsPerMillisecond = nanoseconds(milliseconds(1)).count(); From 1c7973e73d9eee6c93a252d7d7bff15b1399b1ce Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 6 Mar 2020 09:24:42 -0500 Subject: [PATCH 7/8] Fix shellcheck warning. --- release/release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release/release.sh b/release/release.sh index bc34781..8649e13 100755 --- a/release/release.sh +++ b/release/release.sh @@ -86,7 +86,7 @@ trap exit_handler EXIT # we make sure it's installed early on so we don't fail after completing part # of the release. We also use 'hub' to do the clone so that the user is asked # to authenticate at the beginning of the process rather than at the end. -if ! which hub > /dev/null; then +if command -v hub > /dev/null; then echo "Can't find 'hub' command" echo "Maybe run: sudo apt install hub" echo "Or build it from https://github.com/github/hub" From 2cb3562df5b62c9fa7f75bb7ba022ecef5c1f3de Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 6 Mar 2020 11:11:07 -0500 Subject: [PATCH 8/8] Address review comments. --- google/cloud/internal/compiler_info.cc | 8 +++---- .../cloud/internal/completion_queue_impl.cc | 2 +- google/cloud/internal/format_time_point.cc | 23 ++++++++---------- google/cloud/internal/parse_rfc3339.cc | 24 +++++++++---------- google/cloud/internal/utility.h | 2 +- google/cloud/log.h | 2 +- google/cloud/testing_util/chrono_literals.h | 12 +++++----- .../testing_util/scoped_environment_test.cc | 2 +- 8 files changed, 36 insertions(+), 39 deletions(-) diff --git a/google/cloud/internal/compiler_info.cc b/google/cloud/internal/compiler_info.cc index 7792d2b..6dafeda 100644 --- a/google/cloud/internal/compiler_info.cc +++ b/google/cloud/internal/compiler_info.cc @@ -81,10 +81,10 @@ std::string CompilerFeatures() { } std::string LanguageVersion() { - constexpr auto kMagicVersionCxx98 = 199711L; - constexpr auto kMagicVersionCxx11 = 201103L; - constexpr auto kMagicVersionCxx14 = 201402L; - constexpr auto kMagicVersionCxx17 = 201703L; + auto constexpr kMagicVersionCxx98 = 199711L; + auto constexpr kMagicVersionCxx11 = 201103L; + auto constexpr kMagicVersionCxx14 = 201402L; + auto constexpr kMagicVersionCxx17 = 201703L; switch (__cplusplus) { case kMagicVersionCxx98: return "1998"; diff --git a/google/cloud/internal/completion_queue_impl.cc b/google/cloud/internal/completion_queue_impl.cc index 18c3709..8e368b2 100644 --- a/google/cloud/internal/completion_queue_impl.cc +++ b/google/cloud/internal/completion_queue_impl.cc @@ -19,7 +19,7 @@ // There is no wait to unblock the gRPC event loop, not even calling Shutdown(), // so we periodically wake up from the loop to check if the application has // shutdown the run. -constexpr std::chrono::milliseconds kLoopTimeout(50); +std::chrono::milliseconds constexpr kLoopTimeout(50); namespace google { namespace cloud { diff --git a/google/cloud/internal/format_time_point.cc b/google/cloud/internal/format_time_point.cc index c319771..db2dbb9 100644 --- a/google/cloud/internal/format_time_point.cc +++ b/google/cloud/internal/format_time_point.cc @@ -30,16 +30,15 @@ std::string FormatFractional(std::chrono::nanoseconds ns) { using std::chrono::milliseconds; using std::chrono::nanoseconds; using std::chrono::seconds; - constexpr int kMaxNanosecondsDigits = 9; + auto constexpr kMaxNanosecondsDigits = 9; auto constexpr kBufferSize = 16; static_assert(kBufferSize > (kMaxNanosecondsDigits // digits + 1 // period - + 1) // NUL terminator - , + + 1), // NUL terminator "Buffer is not large enough for printing nanoseconds"); - constexpr int kNanosecondsPerMillisecond = + auto constexpr kNanosecondsPerMillisecond = nanoseconds(milliseconds(1)).count(); - constexpr int kMillisecondsPerSecond = milliseconds(seconds(1)).count(); + auto constexpr kMillisecondsPerSecond = milliseconds(seconds(1)).count(); std::array buffer{}; // If the fractional seconds can be just expressed as milliseconds, do that, @@ -79,7 +78,7 @@ namespace cloud { inline namespace GOOGLE_CLOUD_CPP_NS { namespace internal { -constexpr int kTimestampFormatSize = 256; +auto constexpr kTimestampFormatSize = 256; static_assert(kTimestampFormatSize > ((4 + 1) // YYYY- + (2 + 1) // MM- + 2 // DD @@ -87,8 +86,7 @@ static_assert(kTimestampFormatSize > ((4 + 1) // YYYY- + (2 + 1) // HH: + (2 + 1) // MM: + 2 // SS - + 1) // Z - , + + 1), // Z "Buffer size not large enough for YYYY-MM-DDTHH:MM:SSZ format"); std::string FormatRfc3339(std::chrono::system_clock::time_point tp) { @@ -118,11 +116,10 @@ std::string FormatV4SignedUrlTimestamp( std::string FormatV4SignedUrlScope(std::chrono::system_clock::time_point tp) { std::tm tm = AsUtcTm(tp); - constexpr int kDateFormatSize = 256; - static_assert(kDateFormatSize > (4 // YYYY - + 2 // MM - + 2) // DD - , + auto constexpr kDateFormatSize = 256; + static_assert(kDateFormatSize > (4 // YYYY + + 2 // MM + + 2), // DD "Buffer size not large enough for YYYYMMDD format"); std::array buffer{}; std::strftime(buffer.data(), buffer.size(), "%Y%m%d", &tm); diff --git a/google/cloud/internal/parse_rfc3339.cc b/google/cloud/internal/parse_rfc3339.cc index 1cd8097..b8beb0b 100644 --- a/google/cloud/internal/parse_rfc3339.cc +++ b/google/cloud/internal/parse_rfc3339.cc @@ -35,11 +35,11 @@ bool IsLeapYear(int year) { return (year % 4 == 0 && (year % 100 != 0 || year % 400 == 0)); } -constexpr int kMonthsInYear = 12; -constexpr int kHoursInDay = 24; -constexpr int kMinutesInHour = +auto constexpr kMonthsInYear = 12; +auto constexpr kHoursInDay = 24; +auto constexpr kMinutesInHour = std::chrono::seconds(std::chrono::minutes(1)).count(); -constexpr int kSecondsInMinute = +auto constexpr kSecondsInMinute = std::chrono::minutes(std::chrono::hours(1)).count(); std::chrono::system_clock::time_point ParseDateTime( @@ -56,8 +56,8 @@ std::chrono::system_clock::time_point ParseDateTime( std::sscanf(buffer, "%4d-%2d-%2d%c%2d:%2d:%2d%n", &year, &month, &day, &date_time_separator, &hours, &minutes, &seconds, &pos); // All the fields up to this point have fixed width, so total width must be: - constexpr int kExpectedWidth = 19; - constexpr int kExpectedFields = 7; + auto constexpr kExpectedWidth = 19; + auto constexpr kExpectedFields = 7; if (count != kExpectedFields || pos != kExpectedWidth) { ReportError(timestamp, "Invalid format for RFC 3339 timestamp detected while parsing" @@ -67,7 +67,7 @@ std::chrono::system_clock::time_point ParseDateTime( ReportError(timestamp, "Invalid date-time separator, expected 'T' or 't'."); } - constexpr std::array kMaxDaysInMonth{ + std::array constexpr kMaxDaysInMonth{ 31, // January 29, // February (non-leap years checked below) 31, // March @@ -81,7 +81,7 @@ std::chrono::system_clock::time_point ParseDateTime( 30, // November 31, // December }; - constexpr int kMkTimeBaseYear = 1900; + auto constexpr kMkTimeBaseYear = 1900; if (month < 1 || month > kMonthsInYear) { ReportError(timestamp, "Out of range month."); } @@ -131,8 +131,8 @@ std::chrono::system_clock::duration ParseFractionalSeconds( if (count != 1) { ReportError(timestamp, "Invalid fractional seconds component."); } - constexpr int kMaxNanosecondDigits = 9; - constexpr int kNanosecondsBase = 10; + auto constexpr kMaxNanosecondDigits = 9; + auto constexpr kNanosecondsBase = 10; // Normalize the fractional seconds to nanoseconds. for (int digits = pos; digits < kMaxNanosecondDigits; ++digits) { fractional_seconds *= kNanosecondsBase; @@ -155,8 +155,8 @@ std::chrono::seconds ParseOffset(char const*& buffer, // Parse the HH:MM offset. int hours, minutes, pos; // NOLINT(readability-isolate-declaration) auto count = std::sscanf(buffer, "%2d:%2d%n", &hours, &minutes, &pos); - constexpr int kExpectedOffsetWidth = 5; - constexpr int kExpectedOffsetFields = 2; + auto constexpr kExpectedOffsetWidth = 5; + auto constexpr kExpectedOffsetFields = 2; if (count != kExpectedOffsetFields || pos != kExpectedOffsetWidth) { ReportError(timestamp, "Invalid timezone offset, expected [+-]HH:MM."); } diff --git a/google/cloud/internal/utility.h b/google/cloud/internal/utility.h index 3f3b3a5..4094815 100644 --- a/google/cloud/internal/utility.h +++ b/google/cloud/internal/utility.h @@ -29,7 +29,7 @@ namespace internal { template struct integer_sequence { typedef T value_type; - static constexpr size_t size() noexcept { return sizeof...(I); } + static std::size_t constexpr size() noexcept { return sizeof...(I); } }; // Reimplementation of C++14 `std::index_sequence`. diff --git a/google/cloud/log.h b/google/cloud/log.h index c072a15..9e4a632 100644 --- a/google/cloud/log.h +++ b/google/cloud/log.h @@ -240,7 +240,7 @@ class LogSink { LogSink(); /// Return true if the severity is enabled at compile time. - constexpr static bool CompileTimeEnabled(Severity level) { + static bool constexpr CompileTimeEnabled(Severity level) { return level >= Severity::GCP_LS_LOWEST_ENABLED; } diff --git a/google/cloud/testing_util/chrono_literals.h b/google/cloud/testing_util/chrono_literals.h index 1fb30d0..8052bf8 100644 --- a/google/cloud/testing_util/chrono_literals.h +++ b/google/cloud/testing_util/chrono_literals.h @@ -24,27 +24,27 @@ namespace cloud { inline namespace GOOGLE_CLOUD_CPP_NS { namespace testing_util { namespace chrono_literals { -constexpr std::chrono::hours operator"" _h(unsigned long long h) { +std::chrono::hours constexpr operator"" _h(unsigned long long h) { return std::chrono::hours(h); } -constexpr std::chrono::minutes operator"" _min(unsigned long long m) { +std::chrono::minutes constexpr operator"" _min(unsigned long long m) { return std::chrono::minutes(m); } -constexpr std::chrono::seconds operator"" _s(unsigned long long s) { +std::chrono::seconds constexpr operator"" _s(unsigned long long s) { return std::chrono::seconds(s); } -constexpr std::chrono::milliseconds operator"" _ms(unsigned long long ms) { +std::chrono::milliseconds constexpr operator"" _ms(unsigned long long ms) { return std::chrono::milliseconds(ms); } -constexpr std::chrono::microseconds operator"" _us(unsigned long long us) { +std::chrono::microseconds constexpr operator"" _us(unsigned long long us) { return std::chrono::microseconds(us); } -constexpr std::chrono::nanoseconds operator"" _ns(unsigned long long ns) { +std::chrono::nanoseconds constexpr operator"" _ns(unsigned long long ns) { return std::chrono::nanoseconds(ns); } diff --git a/google/cloud/testing_util/scoped_environment_test.cc b/google/cloud/testing_util/scoped_environment_test.cc index e8fddf5..30c045c 100644 --- a/google/cloud/testing_util/scoped_environment_test.cc +++ b/google/cloud/testing_util/scoped_environment_test.cc @@ -23,7 +23,7 @@ inline namespace GOOGLE_CLOUD_CPP_NS { namespace testing_util { namespace { -constexpr auto kVarName = "SCOPED_ENVIRONMENT_TEST"; +auto constexpr kVarName = "SCOPED_ENVIRONMENT_TEST"; TEST(ScopedEnvironment, SetOverSet) { ScopedEnvironment env_outer(kVarName, "foo");