diff --git a/common/check_test.cpp b/common/check_test.cpp index d17ff5cffa4d..810860aee0bf 100644 --- a/common/check_test.cpp +++ b/common/check_test.cpp @@ -6,7 +6,8 @@ #include -namespace Carbon { +namespace Carbon::Testing { +namespace { TEST(CheckTest, CheckTrue) { CHECK(true); } @@ -50,4 +51,5 @@ TEST(ErrorTest, FatalNoReturnRequired) { "FATAL failure at common/check_test.cpp:.+: msg\n"); } -} // namespace Carbon +} // namespace +} // namespace Carbon::Testing diff --git a/common/indirect_value_test.cpp b/common/indirect_value_test.cpp index 9263578e3dbd..345984d80867 100644 --- a/common/indirect_value_test.cpp +++ b/common/indirect_value_test.cpp @@ -8,7 +8,7 @@ #include -namespace Carbon { +namespace Carbon::Testing { namespace { TEST(IndirectValueTest, ConstAccess) { @@ -126,4 +126,4 @@ TEST(IndirectValueTest, IncompleteType) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/common/string_helpers.cpp b/common/string_helpers.cpp index 88efbb59c760..2d66f4dae345 100644 --- a/common/string_helpers.cpp +++ b/common/string_helpers.cpp @@ -13,13 +13,11 @@ namespace Carbon { -namespace { - -constexpr llvm::StringRef TripleQuotes = "\"\"\""; -constexpr llvm::StringRef HorizontalWhitespaceChars = " \t"; +static constexpr llvm::StringRef TripleQuotes = "\"\"\""; +static constexpr llvm::StringRef HorizontalWhitespaceChars = " \t"; // Carbon only takes uppercase hex input. -auto FromHex(char c) -> std::optional { +static auto FromHex(char c) -> std::optional { if (c >= '0' && c <= '9') { return c - '0'; } @@ -30,12 +28,10 @@ auto FromHex(char c) -> std::optional { } // Creates an error instance with the specified `message`. -llvm::Expected MakeError(llvm::Twine message) { +static auto MakeError(llvm::Twine message) -> llvm::Expected { return llvm::createStringError(llvm::inconvertibleErrorCode(), message); } -} // namespace - auto UnescapeStringLiteral(llvm::StringRef source, bool is_block_string) -> std::optional { std::string ret; diff --git a/common/string_helpers_test.cpp b/common/string_helpers_test.cpp index 24b1bf7b7592..321830633df9 100644 --- a/common/string_helpers_test.cpp +++ b/common/string_helpers_test.cpp @@ -15,7 +15,7 @@ using ::llvm::toString; using ::testing::Eq; using ::testing::Optional; -namespace Carbon { +namespace Carbon::Testing { namespace { TEST(UnescapeStringLiteral, Valid) { @@ -197,4 +197,4 @@ TEST(ParseBlockStringLiteral, OkMultipleSlashes) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/docs/project/cpp_style_guide.md b/docs/project/cpp_style_guide.md index 005e13ace7fd..4874ae2c47e8 100644 --- a/docs/project/cpp_style_guide.md +++ b/docs/project/cpp_style_guide.md @@ -141,6 +141,15 @@ these. necessary to create a scope for a variable. - Always break the line immediately after an open brace except for empty loop bodies. +- For + [internal linkage](https://google.github.io/styleguide/cppguide.html#Internal_Linkage) + of definitions of functions and variables, prefer `static` over anonymous + namespaces. `static` minimizes the context necessary to notice the internal + linkage of a definition. + - Anonymous namespaces are still necessary for classes and enums. + - Tests are an exception and should typically be wrapped with + `namespace Carbon::Testing { namespace { ... } }` to keep everything + internal. ### Copyable and movable types diff --git a/executable_semantics/ast/ast_test_matchers_test.cpp b/executable_semantics/ast/ast_test_matchers_test.cpp index 6ad803a70235..3066e90af828 100644 --- a/executable_semantics/ast/ast_test_matchers_test.cpp +++ b/executable_semantics/ast/ast_test_matchers_test.cpp @@ -13,7 +13,7 @@ #include "executable_semantics/ast/statement.h" #include "executable_semantics/common/arena.h" -namespace Carbon { +namespace Carbon::Testing { namespace { using ::testing::_; @@ -159,4 +159,4 @@ TEST(ASTDeclarationsTest, BasicUsage) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/executable_semantics/ast/expression_test.cpp b/executable_semantics/ast/expression_test.cpp index a4d8ed9939bb..131ca9142feb 100644 --- a/executable_semantics/ast/expression_test.cpp +++ b/executable_semantics/ast/expression_test.cpp @@ -13,7 +13,7 @@ #include "executable_semantics/common/arena.h" #include "llvm/Support/Casting.h" -namespace Carbon { +namespace Carbon::Testing { namespace { using llvm::cast; @@ -135,4 +135,4 @@ TEST_F(ExpressionTest, BinaryAsTuple) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/executable_semantics/ast/pattern_test.cpp b/executable_semantics/ast/pattern_test.cpp index e52cb35503d8..ffa2fa7fb24f 100644 --- a/executable_semantics/ast/pattern_test.cpp +++ b/executable_semantics/ast/pattern_test.cpp @@ -12,7 +12,7 @@ #include "executable_semantics/common/arena.h" #include "llvm/Support/Casting.h" -namespace Carbon { +namespace Carbon::Testing { namespace { using llvm::cast; @@ -129,4 +129,4 @@ TEST_F(PatternTest, BinaryAsTuplePattern) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/executable_semantics/common/error_test.cpp b/executable_semantics/common/error_test.cpp index 46a7daae82cd..1d4d9bce3a49 100644 --- a/executable_semantics/common/error_test.cpp +++ b/executable_semantics/common/error_test.cpp @@ -6,7 +6,7 @@ #include -namespace Carbon { +namespace Carbon::Testing { namespace { TEST(ErrorTest, FatalProgramError) { @@ -30,4 +30,4 @@ TEST(ErrorTest, FatalProgramErrorLine) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/executable_semantics/interpreter/value.cpp b/executable_semantics/interpreter/value.cpp index 0ca55b6a449e..5effada4841f 100644 --- a/executable_semantics/interpreter/value.cpp +++ b/executable_semantics/interpreter/value.cpp @@ -27,10 +27,8 @@ auto StructValue::FindField(const std::string& name) const return std::nullopt; } -namespace { - -auto GetMember(Nonnull arena, Nonnull v, - const std::string& f, SourceLocation source_loc) +static auto GetMember(Nonnull arena, Nonnull v, + const std::string& f, SourceLocation source_loc) -> Nonnull { switch (v->kind()) { case Value::Kind::StructValue: { @@ -62,8 +60,6 @@ auto GetMember(Nonnull arena, Nonnull v, } } -} // namespace - auto Value::GetField(Nonnull arena, const FieldPath& path, SourceLocation source_loc) const -> Nonnull { Nonnull value(this); @@ -73,13 +69,11 @@ auto Value::GetField(Nonnull arena, const FieldPath& path, return value; } -namespace { - -auto SetFieldImpl(Nonnull arena, Nonnull value, - std::vector::const_iterator path_begin, - std::vector::const_iterator path_end, - Nonnull field_value, SourceLocation source_loc) - -> Nonnull { +static auto SetFieldImpl(Nonnull arena, Nonnull value, + std::vector::const_iterator path_begin, + std::vector::const_iterator path_end, + Nonnull field_value, + SourceLocation source_loc) -> Nonnull { if (path_begin == path_end) { return field_value; } @@ -120,8 +114,6 @@ auto SetFieldImpl(Nonnull arena, Nonnull value, } } -} // namespace - auto Value::SetField(Nonnull arena, const FieldPath& path, Nonnull field_value, SourceLocation source_loc) const -> Nonnull { diff --git a/executable_semantics/syntax/parse_test.cpp b/executable_semantics/syntax/parse_test.cpp index 73b12fdaae9e..9c196ff0dd2f 100644 --- a/executable_semantics/syntax/parse_test.cpp +++ b/executable_semantics/syntax/parse_test.cpp @@ -12,7 +12,7 @@ #include "executable_semantics/common/arena.h" -namespace Carbon { +namespace Carbon::Testing { namespace { static constexpr std::string_view FileContents = R"( @@ -30,4 +30,4 @@ TEST(ParseTest, ParseFromString) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/executable_semantics/syntax/unimplemented_example_test.cpp b/executable_semantics/syntax/unimplemented_example_test.cpp index 64f8397931bb..3426da22ca69 100644 --- a/executable_semantics/syntax/unimplemented_example_test.cpp +++ b/executable_semantics/syntax/unimplemented_example_test.cpp @@ -9,7 +9,7 @@ #include "executable_semantics/syntax/parse.h" #include "executable_semantics/syntax/parse_test_matchers.h" -namespace Carbon { +namespace Carbon::Testing { namespace { using ::testing::ElementsAre; @@ -33,4 +33,4 @@ TEST(UnimplementedExampleTest, VerifyPrecedence) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp b/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp index d08de18f68b1..5fe8a248f20c 100644 --- a/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp +++ b/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp @@ -6,7 +6,7 @@ #include "migrate_cpp/cpp_refactoring/matcher_test_base.h" -namespace Carbon { +namespace Carbon::Testing { namespace { class FnInserterTest : public MatcherTestBase {}; @@ -90,4 +90,4 @@ TEST_F(FnInserterTest, LegacyReturn) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/migrate_cpp/cpp_refactoring/for_range_test.cpp b/migrate_cpp/cpp_refactoring/for_range_test.cpp index c0da25a3b20e..4a3150c8f610 100644 --- a/migrate_cpp/cpp_refactoring/for_range_test.cpp +++ b/migrate_cpp/cpp_refactoring/for_range_test.cpp @@ -6,7 +6,7 @@ #include "migrate_cpp/cpp_refactoring/matcher_test_base.h" -namespace Carbon { +namespace Carbon::Testing { namespace { class ForRangeTest : public MatcherTestBase {}; @@ -49,4 +49,4 @@ TEST_F(ForRangeTest, NoSpace) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing diff --git a/migrate_cpp/cpp_refactoring/matcher_test_base.h b/migrate_cpp/cpp_refactoring/matcher_test_base.h index 9d0e70565721..f281c62b45ec 100644 --- a/migrate_cpp/cpp_refactoring/matcher_test_base.h +++ b/migrate_cpp/cpp_refactoring/matcher_test_base.h @@ -13,7 +13,7 @@ #include "clang/Tooling/Tooling.h" #include "migrate_cpp/cpp_refactoring/matcher_manager.h" -namespace Carbon { +namespace Carbon::Testing { // Matcher test framework. template @@ -61,6 +61,6 @@ class MatcherTestBase : public ::testing::Test { MatcherManager matchers; }; -} // namespace Carbon +} // namespace Carbon::Testing #endif // MIGRATE_CPP_CPP_REFACTORING_MATCHER_TEST_BASE_H_ diff --git a/migrate_cpp/cpp_refactoring/var_decl_test.cpp b/migrate_cpp/cpp_refactoring/var_decl_test.cpp index 8482cc2e1dad..6376e2db703b 100644 --- a/migrate_cpp/cpp_refactoring/var_decl_test.cpp +++ b/migrate_cpp/cpp_refactoring/var_decl_test.cpp @@ -6,7 +6,7 @@ #include "migrate_cpp/cpp_refactoring/matcher_test_base.h" -namespace Carbon { +namespace Carbon::Testing { namespace { class VarDeclTest : public MatcherTestBase {}; @@ -254,4 +254,4 @@ TEST_F(VarDeclTest, Template) { } } // namespace -} // namespace Carbon +} // namespace Carbon::Testing