From f6248a4b6fdca4ee7dae1599e40db7576851f40f Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Thu, 20 Oct 2022 09:02:14 -0700 Subject: [PATCH] Manual clang-tidy fixes (#2319) Note, not trying to address every last error, just some obvious/easy ones. ``` /usr/local/google/home/jperkins/dev/carbon-lang/common/string_helpers.cpp:200:13: warning: prefer transparent functors 'less_equal<>' [modernize-use-transparent-functors] auto le = std::less_equal(); ^ /usr/local/google/home/jperkins/dev/carbon-lang/toolchain/semantics/nodes/function.h:22:45: warning: pass by value and use std::move [modernize-pass-by-value] Function(ParseTree::Node node, NodeId id, llvm::SmallVector body) ^ /usr/local/google/home/jperkins/dev/carbon-lang/explorer/ast/declaration.cpp:230:14: warning: static member accessed through instance [readability-static-accessed-through-instance] return cast(declaration).name(); ^ /usr/local/google/home/jperkins/dev/carbon-lang/explorer/interpreter/type_checker.cpp:1104:33: warning: std::move of the variable 'impl' of the trivially-copyable type 'ConstraintType::ImplConstraint' has no effect [performance-move-const-arg] impl_constraints_.push_back(std::move(impl)); ^ /usr/local/google/home/jperkins/dev/carbon-lang/explorer/interpreter/type_checker.cpp:1145:36: warning: std::move of the variable 'rewrite' of the trivially-copyable type 'ConstraintType::RewriteConstraint' has no effect [performance-move-const-arg] rewrite_constraints_.push_back(std::move(rewrite)); ^ /usr/local/google/home/jperkins/dev/carbon-lang/explorer/interpreter/type_checker.cpp:1156:32: warning: std::move of the variable 'context' of the trivially-copyable type 'ConstraintType::LookupContext' has no effect [performance-move-const-arg] lookup_contexts_.push_back(std::move(context)); ^ /usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/fuzzverter.cpp:74:42: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment] /*trace=*/false); ^ ./explorer/syntax/parse.h:19:17: note: 'parser_debug' declared here bool parser_debug) -> ErrorOr; ^ /usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/proto_to_carbon_test.cpp:34:55: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment] const ErrorOr ast = Carbon::Parse(&arena, f, /*trace=*/false); ^ ./explorer/syntax/parse.h:19:17: note: 'parser_debug' declared here bool parser_debug) -> ErrorOr; ^ /usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/proto_to_carbon_test.cpp:42:41: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment] &arena, f, source_from_proto, /*trace=*/false); ^ ./explorer/syntax/parse.h:25:59: note: 'parser_debug' declared here std::string_view file_contents, bool parser_debug) ^ /usr/local/google/home/jperkins/dev/carbon-lang/explorer/syntax/parse_test.cpp:27:60: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment] ParseFromString(&arena, "file.carbon", FileContents, /*trace=*/false); ^ ./explorer/syntax/parse.h:25:59: note: 'parser_debug' declared here std::string_view file_contents, bool parser_debug) ^ /usr/local/google/home/jperkins/dev/carbon-lang/migrate_cpp/cpp_refactoring/var_decl.cpp:57:58: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead [performance-inefficient-string-concatenation] segments.push_back({type_loc_class, qual_str + " " + range_str}); ^ /usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/ast_to_proto_test.cpp:105:55: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment] const ErrorOr ast = Carbon::Parse(&arena, f, /*trace=*/false); ^ ./explorer/syntax/parse.h:19:17: note: 'parser_debug' declared here bool parser_debug) -> ErrorOr; ^ ``` --- common/string_helpers.cpp | 2 +- explorer/ast/declaration.cpp | 2 +- explorer/fuzzing/ast_to_proto_test.cpp | 2 +- explorer/fuzzing/fuzzverter.cpp | 2 +- explorer/fuzzing/proto_to_carbon_test.cpp | 4 ++-- explorer/interpreter/type_checker.cpp | 6 +++--- explorer/syntax/parse_test.cpp | 4 ++-- migrate_cpp/cpp_refactoring/BUILD | 1 + migrate_cpp/cpp_refactoring/var_decl.cpp | 4 +++- toolchain/semantics/nodes/function.h | 2 +- 10 files changed, 16 insertions(+), 13 deletions(-) diff --git a/common/string_helpers.cpp b/common/string_helpers.cpp index 7548d574c407..583c3dc7b19a 100644 --- a/common/string_helpers.cpp +++ b/common/string_helpers.cpp @@ -197,7 +197,7 @@ auto ParseBlockStringLiteral(llvm::StringRef source, const int hashtag_num) } auto StringRefContainsPointer(llvm::StringRef ref, const char* ptr) -> bool { - auto le = std::less_equal(); + auto le = std::less_equal<>(); return le(ref.begin(), ptr) && le(ptr, ref.end()); } diff --git a/explorer/ast/declaration.cpp b/explorer/ast/declaration.cpp index cf5d16334c78..8e02ff5a113f 100644 --- a/explorer/ast/declaration.cpp +++ b/explorer/ast/declaration.cpp @@ -227,7 +227,7 @@ auto GetName(const Declaration& declaration) case DeclarationKind::ImplDeclaration: return std::nullopt; case DeclarationKind::SelfDeclaration: - return cast(declaration).name(); + return SelfDeclaration::name(); case DeclarationKind::AliasDeclaration: { return cast(declaration).name(); } diff --git a/explorer/fuzzing/ast_to_proto_test.cpp b/explorer/fuzzing/ast_to_proto_test.cpp index 9e58e5e388da..aa0bfb296cd4 100644 --- a/explorer/fuzzing/ast_to_proto_test.cpp +++ b/explorer/fuzzing/ast_to_proto_test.cpp @@ -102,7 +102,7 @@ TEST(AstToProtoTest, SetsAllProtoFields) { Carbon::Fuzzing::CompilationUnit merged_proto; for (const llvm::StringRef f : *carbon_files) { Carbon::Arena arena; - const ErrorOr ast = Carbon::Parse(&arena, f, /*trace=*/false); + const ErrorOr ast = Carbon::Parse(&arena, f, /*parser_debug=*/false); if (ast.ok()) { merged_proto.MergeFrom(AstToProto(*ast)); } diff --git a/explorer/fuzzing/fuzzverter.cpp b/explorer/fuzzing/fuzzverter.cpp index 58313d2510af..38d318316f4c 100644 --- a/explorer/fuzzing/fuzzverter.cpp +++ b/explorer/fuzzing/fuzzverter.cpp @@ -71,7 +71,7 @@ static auto CarbonToTextProto(std::string_view input_file_name, -> ErrorOr { Carbon::Arena arena; const ErrorOr ast = Carbon::Parse(&arena, input_file_name, - /*trace=*/false); + /*parser_debug=*/false); if (!ast.ok()) { return ErrorBuilder() << "Parsing failed: " << ast.error().message(); } diff --git a/explorer/fuzzing/proto_to_carbon_test.cpp b/explorer/fuzzing/proto_to_carbon_test.cpp index 7d51d8b2eba2..34808b3bcdc6 100644 --- a/explorer/fuzzing/proto_to_carbon_test.cpp +++ b/explorer/fuzzing/proto_to_carbon_test.cpp @@ -31,7 +31,7 @@ TEST(ProtoToCarbonTest, Roundtrip) { int parsed_ok_count = 0; for (const llvm::StringRef f : *carbon_files) { Carbon::Arena arena; - const ErrorOr ast = Carbon::Parse(&arena, f, /*trace=*/false); + const ErrorOr ast = Carbon::Parse(&arena, f, /*parser_debug=*/false); if (ast.ok()) { ++parsed_ok_count; const std::string source_from_proto = ProtoToCarbon(AstToProto(*ast)); @@ -39,7 +39,7 @@ TEST(ProtoToCarbonTest, Roundtrip) { << "Carbon file: " << f << ", source from proto:\n" << source_from_proto); const ErrorOr ast_from_proto = Carbon::ParseFromString( - &arena, f, source_from_proto, /*trace=*/false); + &arena, f, source_from_proto, /*parser_debug=*/false); if (ast_from_proto.ok()) { EXPECT_EQ(AstToString(*ast), AstToString(*ast_from_proto)); diff --git a/explorer/interpreter/type_checker.cpp b/explorer/interpreter/type_checker.cpp index 0dde5aa54336..bc076516828b 100644 --- a/explorer/interpreter/type_checker.cpp +++ b/explorer/interpreter/type_checker.cpp @@ -1101,7 +1101,7 @@ class TypeChecker::ConstraintTypeBuilder { return i; } } - impl_constraints_.push_back(std::move(impl)); + impl_constraints_.push_back(impl); return impl_constraints_.size() - 1; } @@ -1142,7 +1142,7 @@ class TypeChecker::ConstraintTypeBuilder { << " " << rewrite.replacement->value(); } } - rewrite_constraints_.push_back(std::move(rewrite)); + rewrite_constraints_.push_back(rewrite); return Success(); } @@ -1153,7 +1153,7 @@ class TypeChecker::ConstraintTypeBuilder { return; } } - lookup_contexts_.push_back(std::move(context)); + lookup_contexts_.push_back(context); } // Adds all the constraints from another constraint type. The given value diff --git a/explorer/syntax/parse_test.cpp b/explorer/syntax/parse_test.cpp index c979f47c2231..fd7eaab5b389 100644 --- a/explorer/syntax/parse_test.cpp +++ b/explorer/syntax/parse_test.cpp @@ -23,8 +23,8 @@ fn Foo() {} TEST(ParseTest, ParseFromString) { Arena arena; - ErrorOr parse_result = - ParseFromString(&arena, "file.carbon", FileContents, /*trace=*/false); + ErrorOr parse_result = ParseFromString( + &arena, "file.carbon", FileContents, /*parser_debug=*/false); ASSERT_TRUE(parse_result.ok()); EXPECT_EQ(parse_result->declarations.size(), 1); } diff --git a/migrate_cpp/cpp_refactoring/BUILD b/migrate_cpp/cpp_refactoring/BUILD index 19e24d650504..91391e72684a 100644 --- a/migrate_cpp/cpp_refactoring/BUILD +++ b/migrate_cpp/cpp_refactoring/BUILD @@ -98,6 +98,7 @@ cc_library( ":matcher", "@llvm-project//clang:ast_matchers", "@llvm-project//clang:type_nodes_gen", + "@llvm-project//llvm:Support", ], ) diff --git a/migrate_cpp/cpp_refactoring/var_decl.cpp b/migrate_cpp/cpp_refactoring/var_decl.cpp index 9a16ffedfd45..2f9fe9ad6be0 100644 --- a/migrate_cpp/cpp_refactoring/var_decl.cpp +++ b/migrate_cpp/cpp_refactoring/var_decl.cpp @@ -5,6 +5,7 @@ #include "migrate_cpp/cpp_refactoring/var_decl.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "llvm/Support/FormatVariadic.h" namespace cam = ::clang::ast_matchers; @@ -54,7 +55,8 @@ auto VarDecl::GetTypeStr(const clang::VarDecl& decl) -> std::string { } else if (range_str.empty()) { segments.push_back({type_loc_class, qual_str}); } else { - segments.push_back({type_loc_class, qual_str + " " + range_str}); + segments.push_back( + {type_loc_class, llvm::formatv("{0} {1}", qual_str, range_str)}); } type_loc = type_loc.getNextTypeLoc(); diff --git a/toolchain/semantics/nodes/function.h b/toolchain/semantics/nodes/function.h index c4bc8a874fa8..8c3d4d292b14 100644 --- a/toolchain/semantics/nodes/function.h +++ b/toolchain/semantics/nodes/function.h @@ -20,7 +20,7 @@ class Function { static constexpr NodeKind Kind = NodeKind::Function; Function(ParseTree::Node node, NodeId id, llvm::SmallVector body) - : node_(node), id_(id), body_(body) {} + : node_(node), id_(id), body_(std::move(body)) {} void Print( llvm::raw_ostream& out, int indent,