diff --git a/.clang-tidy b/.clang-tidy index fec217e3af72d..58eac2144fbf9 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -29,12 +29,15 @@ Checks: # Disabled due to the implied style choices. - '-modernize-return-braced-init-list' - '-modernize-use-default-member-init' + - '-modernize-use-integer-sign-comparison' - '-modernize-use-emplace' + - '-readability-avoid-nested-conditional-operator' - '-readability-convert-member-functions-to-static' - '-readability-else-after-return' - '-readability-identifier-length' - '-readability-implicit-bool-conversion' - '-readability-make-member-function-const' + - '-readability-math-missing-parentheses' - '-readability-static-definition-in-anonymous-namespace' - '-readability-use-anyofallof' diff --git a/common/hashing.h b/common/hashing.h index ed4808147b45f..c101a68e651c3 100644 --- a/common/hashing.h +++ b/common/hashing.h @@ -591,6 +591,7 @@ inline auto MapToRawDataType(const T& value) -> const T& { // This overload should never be selected for `std::nullptr_t`, so // static_assert to get some better compiler error messages. static_assert(!std::same_as); + // NOLINTNEXTLINE(bugprone-return-const-ref-from-parameter) return value; } inline auto MapToRawDataType(std::nullptr_t /*value*/) -> const void* { diff --git a/common/indirect_value_test.cpp b/common/indirect_value_test.cpp index d9a538b8d4d48..e48121fa7db60 100644 --- a/common/indirect_value_test.cpp +++ b/common/indirect_value_test.cpp @@ -85,6 +85,7 @@ TEST(IndirectValueTest, MutableArrow) { TEST(IndirectValueTest, CopyConstruct) { IndirectValue v1; + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) auto v2 = v1; EXPECT_EQ(v1->state, "default constructed"); EXPECT_EQ(v2->state, "copy constructed"); diff --git a/common/raw_hashtable_metadata_group_benchmark.cpp b/common/raw_hashtable_metadata_group_benchmark.cpp index b1fa2326dd5fc..c51a6eb359c41 100644 --- a/common/raw_hashtable_metadata_group_benchmark.cpp +++ b/common/raw_hashtable_metadata_group_benchmark.cpp @@ -189,6 +189,7 @@ const auto bench_metadata = BuildBenchMetadata(); // group. template static void BM_LoadMatch(benchmark::State& s) { + // NOLINTNEXTLINE(google-readability-casting): Same as on `bench_metadata`. BenchMetadata bm = bench_metadata[0]; // We want to make the index used by the next iteration of the benchmark have diff --git a/scripts/run_clang_tidy.py b/scripts/run_clang_tidy.py deleted file mode 100755 index e4e1d0bce8d4c..0000000000000 --- a/scripts/run_clang_tidy.py +++ /dev/null @@ -1,61 +0,0 @@ -#!/usr/bin/env python3 - -"""Runs clang-tidy over all Carbon files.""" - -__copyright__ = """ -Part of the Carbon Language project, under the Apache License v2.0 with LLVM -Exceptions. See /LICENSE for license information. -SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -""" - -import argparse -import os -import re -import subprocess - -from pathlib import Path - - -def main() -> None: - parser = argparse.ArgumentParser(__doc__) - # Copied from run-clang-tidy.py for forwarding. - parser.add_argument("-fix", action="store_true", help="Apply fix-its") - # Local flags. - parser.add_argument("files", nargs="*", help="Files to fix") - parsed_args = parser.parse_args() - - # If files are passed in, resolve them; otherwise, add a path filter. - if parsed_args.files: - files = [str(Path(f).resolve()) for f in parsed_args.files] - else: - files = ["^(?!.*/(bazel-|third_party)).*$"] - - # Set the repo root as the working directory. - os.chdir(Path(__file__).parents[1]) - # Ensure create_compdb has been run. - subprocess.check_call(["./scripts/create_compdb.py"]) - - # Use the run-clang-tidy version that should be with the rest of the clang - # toolchain. This exposes us to version skew with user-installed clang - # versions, but avoids version skew between the script and clang-tidy - # itself. - with Path( - "./external/_main~clang_toolchain_extension~bazel_cc_toolchain/" - "clang_detected_variables.bzl" - ).open() as f: - clang_vars = f.read() - clang_bindir_match = re.search(r"clang_bindir = \"(.*)\"", clang_vars) - assert clang_bindir_match is not None, clang_vars - - args = [str(Path(clang_bindir_match[1]).joinpath("run-clang-tidy"))] - - # Forward flags. - if parsed_args.fix: - args.append("-fix") - - # Run clang-tidy from clang-tools-extra. - exit(subprocess.call(args + files)) - - -if __name__ == "__main__": - main() diff --git a/testing/base/source_gen.h b/testing/base/source_gen.h index 6b1ef07072ca4..ccd48693ff169 100644 --- a/testing/base/source_gen.h +++ b/testing/base/source_gen.h @@ -54,7 +54,7 @@ namespace Carbon::Testing { // identifiers that should be generalized. class SourceGen { public: - enum class Language { + enum class Language : uint8_t { Carbon, Cpp, }; diff --git a/testing/file_test/autoupdate.h b/testing/file_test/autoupdate.h index cc206e48739a5..6b20581f58e0a 100644 --- a/testing/file_test/autoupdate.h +++ b/testing/file_test/autoupdate.h @@ -105,7 +105,9 @@ class FileTestAutoupdater { }; // A CHECK line which is integrated into autoupdate output. - class CheckLine : public FileTestLineBase { + // + // `final` because we use pointer arithmetic on this type. + class CheckLine final : public FileTestLineBase { public: // RE2 is passed by a pointer because it doesn't support std::optional. explicit CheckLine(FileAndLineNumber file_and_line_number, std::string line) diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index c32ec573d899a..ef34bd1c7cbe2 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -99,7 +99,7 @@ static auto CompareFailPrefix(llvm::StringRef filename, bool success) -> void { } // Modes for GetBazelCommand. -enum class BazelMode { +enum class BazelMode : uint8_t { Autoupdate, Dump, Test, @@ -1081,9 +1081,9 @@ static auto Main(int argc, char** argv) -> int { llvm::errs() << "\nDone!\n"; return EXIT_SUCCESS; } else { - for (llvm::StringRef test_name : tests) { + for (const std::string& test_name : tests) { testing::RegisterTest( - test_factory.name, test_name.data(), nullptr, test_name.data(), + test_factory.name, test_name.c_str(), nullptr, test_name.c_str(), __FILE__, __LINE__, [&test_factory, &exe_path, test_name = test_name]() { return test_factory.factory_fn(exe_path, nullptr, test_name); diff --git a/testing/file_test/line.h b/testing/file_test/line.h index 6ceb45c25b80d..33c78127433e0 100644 --- a/testing/file_test/line.h +++ b/testing/file_test/line.h @@ -31,7 +31,9 @@ class FileTestLineBase : public Printable { }; // A line in the original file test. -class FileTestLine : public FileTestLineBase { +// +// `final` because we use pointer arithmetic on this type. +class FileTestLine final : public FileTestLineBase { public: explicit FileTestLine(int file_number, int line_number, llvm::StringRef line) : FileTestLineBase(file_number, line_number), line_(line) {} diff --git a/toolchain/check/param_and_arg_refs_stack.h b/toolchain/check/param_and_arg_refs_stack.h index 44b28e931d41f..41f631482af82 100644 --- a/toolchain/check/param_and_arg_refs_stack.h +++ b/toolchain/check/param_and_arg_refs_stack.h @@ -73,7 +73,7 @@ class ParamAndArgRefsStack { // Prints the stack for a stack dump. auto PrintForStackDump(int indent, llvm::raw_ostream& output) const -> void { - return stack_.PrintForStackDump(indent, output); + stack_.PrintForStackDump(indent, output); } private: diff --git a/toolchain/check/pattern_match.cpp b/toolchain/check/pattern_match.cpp index b9afe7348bb3e..b2b1943a488b2 100644 --- a/toolchain/check/pattern_match.cpp +++ b/toolchain/check/pattern_match.cpp @@ -35,7 +35,7 @@ static auto GetPrettyName(Context& context, ParamPattern param_pattern) namespace { // Selects between the different kinds of pattern matching. -enum class MatchKind { +enum class MatchKind : uint8_t { // Caller pattern matching occurs on the caller side of a function call, and // is responsible for matching the argument expression against the portion // of the pattern above the ParamPattern insts. diff --git a/toolchain/driver/compile_benchmark.cpp b/toolchain/driver/compile_benchmark.cpp index f31ea9c367902..703ff6b73d347 100644 --- a/toolchain/driver/compile_benchmark.cpp +++ b/toolchain/driver/compile_benchmark.cpp @@ -54,7 +54,7 @@ class CompileBenchmark { }; // An enumerator used to select compilation phases to benchmark. -enum class Phase { +enum class Phase : uint8_t { Lex, Parse, Check, diff --git a/toolchain/language_server/server.cpp b/toolchain/language_server/server.cpp index 7847f317e407d..b2553992c2266 100644 --- a/toolchain/language_server/server.cpp +++ b/toolchain/language_server/server.cpp @@ -30,13 +30,13 @@ Server::Server(std::FILE* input_stream, llvm::raw_ostream& output_stream) auto Server::Run() -> ErrorOr { llvm::Error err = transport_->loop(*this); - if (err.success()) { - return Success(); - } else { + if (err) { std::string str; llvm::raw_string_ostream out(str); out << err; return Error(str); + } else { + return Success(); } } diff --git a/toolchain/lex/lex.cpp b/toolchain/lex/lex.cpp index 390619fdab787..e1877ad992a70 100644 --- a/toolchain/lex/lex.cpp +++ b/toolchain/lex/lex.cpp @@ -660,6 +660,7 @@ static auto DispatchNext(Lexer& lexer, llvm::StringRef source_text, // that because this is a must-tail return, this cannot fail to tail-call // and will not grow the stack. This is in essence a loop with dynamic // tail dispatch to the next stage of the loop. + // NOLINTNEXTLINE(readability-avoid-return-with-void-value): For musttail. [[clang::musttail]] return DispatchTable[static_cast( source_text[position])](lexer, source_text, position); } diff --git a/toolchain/lex/tokenized_buffer_benchmark.cpp b/toolchain/lex/tokenized_buffer_benchmark.cpp index 5c91ab4e20456..129a54b060cd3 100644 --- a/toolchain/lex/tokenized_buffer_benchmark.cpp +++ b/toolchain/lex/tokenized_buffer_benchmark.cpp @@ -610,6 +610,7 @@ template auto BasicDispatch(ssize_t& index, const char* text, char* buffer) -> void { *buffer = text[index]; ++index; + // NOLINTNEXTLINE(readability-avoid-return-with-void-value): For musttail. [[clang::musttail]] return Table[static_cast(text[index])]( index, text, buffer); } @@ -620,6 +621,7 @@ auto SpecializedDispatch(ssize_t& index, const char* text, char* buffer) CARBON_CHECK(C == text[index]); *buffer = C; ++index; + // NOLINTNEXTLINE(readability-avoid-return-with-void-value): For musttail. [[clang::musttail]] return Table[static_cast(text[index])]( index, text, buffer); } diff --git a/toolchain/lex/tokenized_buffer_test.cpp b/toolchain/lex/tokenized_buffer_test.cpp index ad8265a2b6fa9..fb642bd240136 100644 --- a/toolchain/lex/tokenized_buffer_test.cpp +++ b/toolchain/lex/tokenized_buffer_test.cpp @@ -1215,14 +1215,16 @@ TEST_F(LexerTest, IndentedComments) { std::string simd_source = source + "\"Add a bunch of padding so that SIMD logic shouldn't hit EOF\""; - auto& simd_buffer = compile_helper_.GetTokenizedBuffer(source); + auto& simd_buffer = compile_helper_.GetTokenizedBuffer(simd_source); ASSERT_FALSE(simd_buffer.has_errors()); EXPECT_THAT(simd_buffer.comments_size(), Eq(1)); } } TEST_F(LexerTest, MultipleComments) { - constexpr llvm::StringLiteral Format = R"( + // TODO: Switch format to `llvm::StringLiteral` if + // `llvm::StringLiteral::c_str` is added. + constexpr char Format[] = R"( {0} {1} @@ -1252,7 +1254,7 @@ x "//\n" "// Valid\n", "// This uses a high indent, which stops SIMD.\n", "//\n"}; - std::string source = llvm::formatv(Format.data(), Comments[0], Comments[1], + std::string source = llvm::formatv(Format, Comments[0], Comments[1], Comments[2], Comments[3], Comments[4]) .str(); diff --git a/toolchain/parse/extract.cpp b/toolchain/parse/extract.cpp index ae4b87d0161dd..686dd2cd1bc6f 100644 --- a/toolchain/parse/extract.cpp +++ b/toolchain/parse/extract.cpp @@ -76,11 +76,13 @@ class NodeExtractor { std::tuple* /*type*/) -> std::optional; // Split out trace logic. The noinline saves a few seconds on compilation. + // TODO: Switch format to `llvm::StringLiteral` if + // `llvm::StringLiteral::c_str` is added. template - [[clang::noinline]] auto MaybeTrace(llvm::StringLiteral format, - ArgT... args) const -> void { + [[clang::noinline]] auto MaybeTrace(const char* format, ArgT... args) const + -> void { if (trace_) { - *trace_ << llvm::formatv(format.data(), args...); + *trace_ << llvm::formatv(format, args...); } } diff --git a/toolchain/parse/handle_import_and_package.cpp b/toolchain/parse/handle_import_and_package.cpp index f926ca6d1443e..a0e26e97ac7d1 100644 --- a/toolchain/parse/handle_import_and_package.cpp +++ b/toolchain/parse/handle_import_and_package.cpp @@ -15,8 +15,8 @@ namespace Carbon::Parse { // Provides common error exiting logic that skips to the semi, if present. static auto OnParseError(Context& context, Context::StateStackEntry state, NodeKind declaration) -> void { - return context.AddNode(declaration, context.SkipPastLikelyEnd(state.token), - /*has_error=*/true); + context.AddNode(declaration, context.SkipPastLikelyEnd(state.token), + /*has_error=*/true); } // Determines whether the specified modifier appears within the introducer of diff --git a/toolchain/sem_ir/function.h b/toolchain/sem_ir/function.h index 46eec6cc20a10..12c8e519d5e98 100644 --- a/toolchain/sem_ir/function.h +++ b/toolchain/sem_ir/function.h @@ -15,7 +15,7 @@ namespace Carbon::SemIR { // Function-specific fields. struct FunctionFields { // Kinds of virtual modifiers that can apply to functions. - enum class VirtualModifier { None, Virtual, Abstract, Impl }; + enum class VirtualModifier : uint8_t { None, Virtual, Abstract, Impl }; // The following members always have values, and do not change throughout the // lifetime of the function. diff --git a/toolchain/sem_ir/name_scope.h b/toolchain/sem_ir/name_scope.h index c9fa8b1f5208a..0de02513db5aa 100644 --- a/toolchain/sem_ir/name_scope.h +++ b/toolchain/sem_ir/name_scope.h @@ -107,7 +107,7 @@ class NameScope : public Printable { auto AddImportIRScope( const std::pair& import_ir_scope) -> void { - return import_ir_scopes_.push_back(import_ir_scope); + import_ir_scopes_.push_back(import_ir_scope); } private: diff --git a/toolchain/sem_ir/singleton_insts.h b/toolchain/sem_ir/singleton_insts.h index 38693bd8a7baf..88af633a273fb 100644 --- a/toolchain/sem_ir/singleton_insts.h +++ b/toolchain/sem_ir/singleton_insts.h @@ -28,16 +28,16 @@ static constexpr std::array SingletonInstKinds = { }; // Returns true if the InstKind is a singleton. -inline constexpr auto IsSingletonInstKind(InstKind kind) -> bool; +constexpr auto IsSingletonInstKind(InstKind kind) -> bool; // Provides the InstId for singleton instructions. These are exposed as // `InstT::SingletonInstId` in `typed_insts.h`. template requires(IsSingletonInstKind(InstKind::Make(Kind))) -inline constexpr auto MakeSingletonInstId() -> InstId; +constexpr auto MakeSingletonInstId() -> InstId; // Returns true if the InstId corresponds to a singleton inst. -inline constexpr auto IsSingletonInstId(InstId id) -> bool { +constexpr auto IsSingletonInstId(InstId id) -> bool { return id.index >= 0 && id.index < static_cast(SingletonInstKinds.size()); } @@ -47,7 +47,7 @@ inline constexpr auto IsSingletonInstId(InstId id) -> bool { namespace Internal { // Returns the index for a singleton instruction, or -1 if it's not a singleton. -inline constexpr auto GetSingletonInstIndex(InstKind kind) -> int32_t { +constexpr auto GetSingletonInstIndex(InstKind kind) -> int32_t { for (int32_t i = 0; i < static_cast(SingletonInstKinds.size()); ++i) { if (SingletonInstKinds[i] == kind) { @@ -59,13 +59,13 @@ inline constexpr auto GetSingletonInstIndex(InstKind kind) -> int32_t { } // namespace Internal -inline constexpr auto IsSingletonInstKind(InstKind kind) -> bool { +constexpr auto IsSingletonInstKind(InstKind kind) -> bool { return Internal::GetSingletonInstIndex(kind) >= 0; } template requires(IsSingletonInstKind(InstKind::Make(Kind))) -inline constexpr auto MakeSingletonInstId() -> InstId { +constexpr auto MakeSingletonInstId() -> InstId { auto index = Internal::GetSingletonInstIndex(InstKind::Make(Kind)); return InstId(index); } diff --git a/toolchain/testing/coverage_helper.h b/toolchain/testing/coverage_helper.h index 135ddb72f7345..cca5bc6ef3101 100644 --- a/toolchain/testing/coverage_helper.h +++ b/toolchain/testing/coverage_helper.h @@ -21,15 +21,17 @@ namespace Carbon::Testing { // // should_be_covered should return false when a kind is either untestable or not // yet tested. +// +// TODO: Switch `kind_pattern` to `llvm::StringLiteral` if +// `llvm::StringLiteral::c_str` is added. template auto TestKindCoverage(const std::string& manifest_path, - llvm::StringLiteral kind_pattern, - llvm::ArrayRef kinds, + const char* kind_pattern, llvm::ArrayRef kinds, llvm::ArrayRef untested_kinds) { std::ifstream manifest_in(manifest_path.c_str()); ASSERT_TRUE(manifest_in.good()); - RE2 kind_re(kind_pattern.data()); + RE2 kind_re(kind_pattern); ASSERT_TRUE(kind_re.ok()) << kind_re.error(); Set covered_kinds; diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index 1983beeaa2245..eae4d78165ae8 100644 --- a/toolchain/testing/file_test.cpp +++ b/toolchain/testing/file_test.cpp @@ -130,7 +130,7 @@ class ToolchainFileTest : public FileTestBase { R"((FileEnd.*column: |FileStart.*line: )( *\d+))"); RE2::Replace(&check_line, file_token_re, R"(\1{{ *\\d+}})"); } else { - return FileTestBase::DoExtraCheckReplacements(check_line); + FileTestBase::DoExtraCheckReplacements(check_line); } }