From 3f9a06aee38820dc8015b79a500896c6ada5032c Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 17 Dec 2024 13:20:37 -0800 Subject: [PATCH] Look at flipping clang-tidy's misc-* to enable-by-default (#4699) I was wondering, instead of treating `misc` differently and enabling specific checks, maybe we can flip that since we actually seem okay with most of the checks? The main check I'm enabling, with significant edits here, is `misc-no-recursion`. But maybe this is helpful to enable, even with the necessary NOLINTs, since we want to avoid recursion in the toolchain? This PR shows some example fixes in subst.cpp (which are more stylistic, since the code shouldn't actually have recursed due to its structure; I think we could remove the warning on TryResolveInst the same way). Some also just don't seem worth fixing, like those in tests files (I didn't see a way to exclude files in .clang-tidy, so instead I'm using NOLINTBEGIN). But I think we might actually want to fix inst_namer, and there's enough in convert that I didn't look closely. Also, I made some protected -> private style fixes based on `misc-non-private-member-variables-in-classes` (this is also how I noticed `class Real` versus `struct Real`). With node_stack, it looks like the `protected` wasn't even used. [Per style](https://google.github.io/styleguide/cppguide.html#Access_Control), data members should be private outside tests. But since we can't trivially exclude `protected` members in tests, I'm turning it off -- I don't view it as offering enough benefit on the whole. migrate_cpp issues are preexisting (I believe we just aren't monitoring it), but changes there make `bazel build --config=clang-tidy -k //...` work cleanly. --------- Co-authored-by: Richard Smith --- .clang-tidy | 18 +++--- common/command_line.cpp | 36 ++++++----- common/command_line.h | 21 ++++--- migrate_cpp/cpp_refactoring/fn_inserter.cpp | 1 + migrate_cpp/cpp_refactoring/for_range.cpp | 1 + migrate_cpp/cpp_refactoring/var_decl.cpp | 2 +- migrate_cpp/rewriter.cpp | 2 + testing/fuzzing/proto_to_carbon.cpp | 5 ++ toolchain/base/value_ids.h | 3 +- toolchain/check/convert.cpp | 6 ++ toolchain/check/import_ref.cpp | 24 +++++-- toolchain/check/subst.cpp | 69 ++++++++++++--------- toolchain/sem_ir/formatter.cpp | 7 +++ toolchain/sem_ir/inst_namer.cpp | 4 ++ toolchain/testing/yaml_test_helpers.cpp | 2 + 15 files changed, 128 insertions(+), 73 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 58eac2144fbf..eaf3b92c0d9b 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -13,20 +13,15 @@ Checks: - '-*' - 'bugprone-*' - 'google-*' + - 'misc-*' - 'modernize-*' - 'performance-*' - 'readability-*' - # `misc` is selectively enabled because we want a minority of them. - - 'misc-definitions-in-headers' - - 'misc-misplaced-const' - - 'misc-redundant-expression' - - 'misc-static-assert' - - 'misc-unconventional-assign-operator' - - 'misc-uniqueptr-reset-release' - - 'misc-unused-*' - # Disabled due to the implied style choices. + - '-misc-const-correctness' + - '-misc-include-cleaner' + - '-misc-use-anonymous-namespace' - '-modernize-return-braced-init-list' - '-modernize-use-default-member-init' - '-modernize-use-integer-sign-comparison' @@ -68,6 +63,11 @@ Checks: - '-google-readability-function-size' # Suggests usernames on TODOs, which we don't want. - '-google-readability-todo' + # Even with `IgnoreClassesWithAllMemberVariablesBeingPublic` to allow structs, + # we use `protected` members in too many tests. + - '-misc-non-private-member-variables-in-classes' + # Overlaps with `-Wno-missing-prototypes`. + - '-misc-use-internal-linkage' # Suggests `std::array`, which we could migrate to, but conflicts with the # status quo. - '-modernize-avoid-c-arrays' diff --git a/common/command_line.cpp b/common/command_line.cpp index 95134bed0aa1..59a893731787 100644 --- a/common/command_line.cpp +++ b/common/command_line.cpp @@ -10,6 +10,10 @@ #include "llvm/ADT/PointerIntPair.h" #include "llvm/Support/FormatVariadic.h" +// Recursion is used for subcommands. This should be okay since recursion is +// limited by command line architecture. +// NOLINTBEGIN(misc-no-recursion) + namespace Carbon::CommandLine { auto operator<<(llvm::raw_ostream& output, ParseResult result) @@ -1311,41 +1315,41 @@ void ArgBuilder::HelpHidden(bool is_help_hidden) { ArgBuilder::ArgBuilder(Arg* arg) : arg_(arg) {} void FlagBuilder::Default(bool flag_value) { - arg_->has_default = true; - arg_->default_flag = flag_value; + arg()->has_default = true; + arg()->default_flag = flag_value; } -void FlagBuilder::Set(bool* flag) { arg_->flag_storage = flag; } +void FlagBuilder::Set(bool* flag) { arg()->flag_storage = flag; } void IntegerArgBuilder::Default(int integer_value) { - arg_->has_default = true; - arg_->default_integer = integer_value; + arg()->has_default = true; + arg()->default_integer = integer_value; } void IntegerArgBuilder::Set(int* integer) { - arg_->is_append = false; - arg_->integer_storage = integer; + arg()->is_append = false; + arg()->integer_storage = integer; } void IntegerArgBuilder::Append(llvm::SmallVectorImpl* sequence) { - arg_->is_append = true; - arg_->integer_sequence = sequence; + arg()->is_append = true; + arg()->integer_sequence = sequence; } void StringArgBuilder::Default(llvm::StringRef string_value) { - arg_->has_default = true; - arg_->default_string = string_value; + arg()->has_default = true; + arg()->default_string = string_value; } void StringArgBuilder::Set(llvm::StringRef* string) { - arg_->is_append = false; - arg_->string_storage = string; + arg()->is_append = false; + arg()->string_storage = string; } void StringArgBuilder::Append( llvm::SmallVectorImpl* sequence) { - arg_->is_append = true; - arg_->string_sequence = sequence; + arg()->is_append = true; + arg()->string_sequence = sequence; } static auto IsValidName(llvm::StringRef name) -> bool { @@ -1525,3 +1529,5 @@ auto Parse(llvm::ArrayRef unparsed_args, } } // namespace Carbon::CommandLine + +// NOLINTEND(misc-no-recursion) diff --git a/common/command_line.h b/common/command_line.h index 2656675f5454..62303b339b10 100644 --- a/common/command_line.h +++ b/common/command_line.h @@ -307,10 +307,13 @@ class ArgBuilder { void MetaAction(T action); protected: - friend CommandBuilder; + friend class CommandBuilder; // `arg` must not be null. explicit ArgBuilder(Arg* arg); + auto arg() -> Arg* { return arg_; } + + private: Arg* arg_; }; @@ -756,7 +759,7 @@ auto OneOfArgBuilder::OneOfValue(llvm::StringRef str, T value) template void OneOfArgBuilder::SetOneOf(const OneOfValueT (&values)[N], T* result) { static_assert(N > 0, "Must include at least one value."); - arg_->is_append = false; + arg()->is_append = false; OneOfImpl( values, [result](T value) { *result = value; }, std::make_index_sequence{}); @@ -766,7 +769,7 @@ template void OneOfArgBuilder::AppendOneOf(const OneOfValueT (&values)[N], llvm::SmallVectorImpl* sequence) { static_assert(N > 0, "Must include at least one value."); - arg_->is_append = true; + arg()->is_append = true; OneOfImpl( values, [sequence](T value) { sequence->push_back(value); }, std::make_index_sequence{}); @@ -792,12 +795,12 @@ void OneOfArgBuilder::OneOfImpl(const OneOfValueT (&input_values)[N], // Directly copy the value strings into a heap-allocated array in the // argument. - new (&arg_->value_strings) + new (&arg()->value_strings) llvm::OwningArrayRef(value_strings); // And build a type-erased action that maps a specific value string to a value // by index. - new (&arg_->value_action) Arg::ValueActionT( + new (&arg()->value_action) Arg::ValueActionT( [values, match](const Arg& arg, llvm::StringRef value_string) -> bool { for (int i : llvm::seq(0, N)) { if (value_string == arg.value_strings[i]) { @@ -810,11 +813,11 @@ void OneOfArgBuilder::OneOfImpl(const OneOfValueT (&input_values)[N], // Fold over all the input values to see if there is a default. if ((input_values[Indices].is_default || ...)) { - CARBON_CHECK(!arg_->is_append, "Can't append default."); + CARBON_CHECK(!arg()->is_append, "Can't append default."); CARBON_CHECK((input_values[Indices].is_default + ... + 0) == 1, "Cannot default more than one value."); - arg_->has_default = true; + arg()->has_default = true; // First build a lambda that configures the default using an index. We'll // call this below, this lambda isn't the one that is stored. @@ -822,12 +825,12 @@ void OneOfArgBuilder::OneOfImpl(const OneOfValueT (&input_values)[N], // Now that we have the desired default index, build a lambda and store it // as the default action. This lambda is stored and so it captures the // necessary information explicitly and by value. - new (&arg_->default_action) + new (&arg()->default_action) Arg::DefaultActionT([value = default_value.value, match](const Arg& /*arg*/) { match(value); }); // Also store the index itself for use when printing help. - arg_->default_value_index = index; + arg()->default_value_index = index; }; // Now we fold across the inputs and in the one case that is the default, we diff --git a/migrate_cpp/cpp_refactoring/fn_inserter.cpp b/migrate_cpp/cpp_refactoring/fn_inserter.cpp index 76558b649a8a..f370ffce160e 100644 --- a/migrate_cpp/cpp_refactoring/fn_inserter.cpp +++ b/migrate_cpp/cpp_refactoring/fn_inserter.cpp @@ -6,6 +6,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" +// NOLINTNEXTLINE(readability-identifier-naming) namespace cam = ::clang::ast_matchers; namespace Carbon { diff --git a/migrate_cpp/cpp_refactoring/for_range.cpp b/migrate_cpp/cpp_refactoring/for_range.cpp index 371b7dc21c42..eef4946fa0fe 100644 --- a/migrate_cpp/cpp_refactoring/for_range.cpp +++ b/migrate_cpp/cpp_refactoring/for_range.cpp @@ -6,6 +6,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" +// NOLINTNEXTLINE(readability-identifier-naming) namespace cam = ::clang::ast_matchers; namespace Carbon { diff --git a/migrate_cpp/cpp_refactoring/var_decl.cpp b/migrate_cpp/cpp_refactoring/var_decl.cpp index 2f9fe9ad6be0..7e586c618ba3 100644 --- a/migrate_cpp/cpp_refactoring/var_decl.cpp +++ b/migrate_cpp/cpp_refactoring/var_decl.cpp @@ -7,6 +7,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "llvm/Support/FormatVariadic.h" +// NOLINTNEXTLINE(readability-identifier-naming) namespace cam = ::clang::ast_matchers; namespace Carbon { @@ -36,7 +37,6 @@ auto VarDecl::GetTypeStr(const clang::VarDecl& decl) -> std::string { auto type_loc = decl.getTypeSourceInfo()->getTypeLoc(); std::vector> segments; while (!type_loc.isNull()) { - std::string text; auto qualifiers = type_loc.getType().getLocalQualifiers(); std::string qual_str; if (!qualifiers.empty()) { diff --git a/migrate_cpp/rewriter.cpp b/migrate_cpp/rewriter.cpp index 55636b74a57e..35f0e7c6b227 100644 --- a/migrate_cpp/rewriter.cpp +++ b/migrate_cpp/rewriter.cpp @@ -297,6 +297,7 @@ auto RewriteBuilder::VisitUnaryOperator(clang::UnaryOperator* expr) -> bool { return true; } +// NOLINTNEXTLINE(misc-no-recursion): Recursion may be okay for migration. auto RewriteBuilder::TraverseFunctionDecl(clang::FunctionDecl* decl) -> bool { clang::TypeLoc return_type_loc = decl->getFunctionTypeLoc().getReturnLoc(); if (!TraverseTypeLoc(return_type_loc)) { @@ -345,6 +346,7 @@ auto RewriteBuilder::TraverseFunctionDecl(clang::FunctionDecl* decl) -> bool { return true; } +// NOLINTNEXTLINE(misc-no-recursion): Recursion may be okay for migration. auto RewriteBuilder::TraverseVarDecl(clang::VarDecl* decl) -> bool { clang::TypeLoc loc = decl->getTypeSourceInfo()->getTypeLoc(); if (!TraverseTypeLoc(loc)) { diff --git a/testing/fuzzing/proto_to_carbon.cpp b/testing/fuzzing/proto_to_carbon.cpp index a9cb40ebc00b..6fef9ad804aa 100644 --- a/testing/fuzzing/proto_to_carbon.cpp +++ b/testing/fuzzing/proto_to_carbon.cpp @@ -12,6 +12,9 @@ #include "llvm/Support/raw_ostream.h" #include "testing/fuzzing/carbon.pb.h" +// This is a test file, so the recursion is okay. +// NOLINTBEGIN(misc-no-recursion) + namespace Carbon { static auto ExpressionToCarbon(const Fuzzing::Expression& expression, @@ -1023,3 +1026,5 @@ auto ParseCarbonTextProto(const std::string& contents) } } // namespace Carbon + +// NOLINTEND(misc-no-recursion) diff --git a/toolchain/base/value_ids.h b/toolchain/base/value_ids.h index 465e8db65257..fc4ee04be61b 100644 --- a/toolchain/base/value_ids.h +++ b/toolchain/base/value_ids.h @@ -21,8 +21,7 @@ namespace Carbon { // // These values are not canonicalized, because we don't expect them to repeat // and don't use them in SemIR values. -class Real : public Printable { - public: +struct Real : public Printable { auto Print(llvm::raw_ostream& output_stream) const -> void { mantissa.print(output_stream, /*isSigned=*/false); output_stream << "*" << (is_decimal ? "10" : "2") << "^" << exponent; diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 507548c3bccb..9e57ebde42cc 100644 --- a/toolchain/check/convert.cpp +++ b/toolchain/check/convert.cpp @@ -22,6 +22,10 @@ #include "toolchain/sem_ir/inst.h" #include "toolchain/sem_ir/typed_insts.h" +// TODO: This contains a lot of recursion. Consider removing it in order to +// prevent accidents. +// NOLINTBEGIN(misc-no-recursion) + namespace Carbon::Check { // Given an initializing expression, find its return slot argument. Returns @@ -1245,3 +1249,5 @@ auto ExprAsType(Context& context, SemIR::LocId loc_id, SemIR::InstId value_id) } } // namespace Carbon::Check + +// NOLINTEND(misc-no-recursion) diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index 26b0b9576b69..fe8a1a1655ff 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -342,6 +342,14 @@ class ImportContext { } protected: + auto pending_generics() -> llvm::SmallVector& { + return pending_generics_; + } + auto pending_specifics() -> llvm::SmallVector& { + return pending_specifics_; + } + + private: Context& context_; SemIR::ImportIRId import_ir_id_; const SemIR::File& import_ir_; @@ -2526,6 +2534,10 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, // Invalid if more has been added to the stack. This is the same as // TryResolveInst, except that it may resolve symbolic constants as canonical // constants instead of as constants associated with a particular generic. +// +// TODO: Consider refactoring the body to a helper in order to eliminate +// recursion. +// NOLINTNEXTLINE(misc-no-recursion) static auto TryResolveInstCanonical(ImportRefResolver& resolver, SemIR::InstId inst_id, SemIR::ConstantId const_id) @@ -2850,7 +2862,7 @@ static auto FinishPendingSpecific(ImportRefResolver& resolver, auto ImportRefResolver::PerformPendingWork() -> void { // Note that the individual Finish steps can add new pending work, so keep // going until we have no more work to do. - while (!pending_generics_.empty() || !pending_specifics_.empty()) { + while (!pending_generics().empty() || !pending_specifics().empty()) { // Process generics in the order that we added them because a later // generic might refer to an earlier one, and the calls to // RebuildGenericEvalBlock assume that the reachable SemIR is in a valid @@ -2858,13 +2870,13 @@ auto ImportRefResolver::PerformPendingWork() -> void { // TODO: Import the generic eval block rather than calling // RebuildGenericEvalBlock to rebuild it so that order doesn't matter. // NOLINTNEXTLINE(modernize-loop-convert) - for (size_t i = 0; i != pending_generics_.size(); ++i) { - FinishPendingGeneric(*this, pending_generics_[i]); + for (size_t i = 0; i != pending_generics().size(); ++i) { + FinishPendingGeneric(*this, pending_generics()[i]); } - pending_generics_.clear(); + pending_generics().clear(); - while (!pending_specifics_.empty()) { - FinishPendingSpecific(*this, pending_specifics_.pop_back_val()); + while (!pending_specifics().empty()) { + FinishPendingSpecific(*this, pending_specifics().pop_back_val()); } } } diff --git a/toolchain/check/subst.cpp b/toolchain/check/subst.cpp index 70766ce9eec7..965d1b77837d 100644 --- a/toolchain/check/subst.cpp +++ b/toolchain/check/subst.cpp @@ -61,6 +61,19 @@ class Worklist { // Pushes the specified operand onto the worklist. static auto PushOperand(Context& context, Worklist& worklist, SemIR::IdKind kind, int32_t arg) -> void { + auto push_block = [&](SemIR::InstBlockId block_id) { + for (auto inst_id : + context.inst_blocks().Get(SemIR::InstBlockId(block_id))) { + worklist.Push(inst_id); + } + }; + + auto push_specific = [&](SemIR::SpecificId specific_id) { + if (specific_id.is_valid()) { + push_block(context.specifics().Get(specific_id).args_id); + } + }; + switch (kind) { case SemIR::IdKind::For: if (SemIR::InstId inst_id(arg); inst_id.is_valid()) { @@ -73,9 +86,7 @@ static auto PushOperand(Context& context, Worklist& worklist, } break; case SemIR::IdKind::For: - for (auto inst_id : context.inst_blocks().Get(SemIR::InstBlockId(arg))) { - worklist.Push(inst_id); - } + push_block(SemIR::InstBlockId(arg)); break; case SemIR::IdKind::For: { for (auto field : @@ -90,18 +101,13 @@ static auto PushOperand(Context& context, Worklist& worklist, } break; case SemIR::IdKind::For: - if (auto specific_id = static_cast(arg); - specific_id.is_valid()) { - PushOperand(context, worklist, SemIR::IdKind::For, - context.specifics().Get(specific_id).args_id.index); - } + push_specific(SemIR::SpecificId(arg)); break; case SemIR::IdKind::For: { const auto& facet_type_info = context.facet_types().Get(SemIR::FacetTypeId(arg)); for (auto interface : facet_type_info.impls_constraints) { - PushOperand(context, worklist, SemIR::IdKind::For, - interface.specific_id.index); + push_specific(interface.specific_id); } for (auto rewrite : facet_type_info.rewrite_constraints) { auto lhs_inst_id = @@ -134,6 +140,25 @@ static auto ExpandOperands(Context& context, Worklist& worklist, // Pops the specified operand from the worklist and returns it. static auto PopOperand(Context& context, Worklist& worklist, SemIR::IdKind kind, int32_t arg) -> int32_t { + auto pop_block_id = [&](SemIR::InstBlockId old_inst_block_id) { + auto size = context.inst_blocks().Get(old_inst_block_id).size(); + SemIR::CopyOnWriteInstBlock new_inst_block(context.sem_ir(), + old_inst_block_id); + for (auto i : llvm::reverse(llvm::seq(size))) { + new_inst_block.Set(i, worklist.Pop()); + } + return new_inst_block.GetCanonical(); + }; + + auto pop_specific = [&](SemIR::SpecificId specific_id) { + if (!specific_id.is_valid()) { + return specific_id; + } + auto& specific = context.specifics().Get(specific_id); + auto args_id = pop_block_id(specific.args_id); + return context.specifics().GetOrAdd(specific.generic_id, args_id); + }; + switch (kind) { case SemIR::IdKind::For: { SemIR::InstId inst_id(arg); @@ -150,14 +175,7 @@ static auto PopOperand(Context& context, Worklist& worklist, SemIR::IdKind kind, return context.GetTypeIdForTypeInst(worklist.Pop()).index; } case SemIR::IdKind::For: { - SemIR::InstBlockId old_inst_block_id(arg); - auto size = context.inst_blocks().Get(old_inst_block_id).size(); - SemIR::CopyOnWriteInstBlock new_inst_block(context.sem_ir(), - old_inst_block_id); - for (auto i : llvm::reverse(llvm::seq(size))) { - new_inst_block.Set(i, worklist.Pop()); - } - return new_inst_block.GetCanonical().index; + return pop_block_id(SemIR::InstBlockId(arg)).index; } case SemIR::IdKind::For: { SemIR::StructTypeFieldsId old_fields_id(arg); @@ -182,15 +200,7 @@ static auto PopOperand(Context& context, Worklist& worklist, SemIR::IdKind kind, return new_type_block.GetCanonical().index; } case SemIR::IdKind::For: { - SemIR::SpecificId specific_id(arg); - if (!specific_id.is_valid()) { - return arg; - } - auto& specific = context.specifics().Get(specific_id); - auto args_id = SemIR::InstBlockId( - PopOperand(context, worklist, SemIR::IdKind::For, - specific.args_id.index)); - return context.specifics().GetOrAdd(specific.generic_id, args_id).index; + return pop_specific(SemIR::SpecificId(arg)).index; } case SemIR::IdKind::For: { const auto& old_facet_type_info = @@ -206,11 +216,8 @@ static auto PopOperand(Context& context, Worklist& worklist, SemIR::IdKind kind, } for (auto i : llvm::reverse( llvm::seq(old_facet_type_info.impls_constraints.size()))) { - auto specific_id = PopOperand( - context, worklist, SemIR::IdKind::For, - old_facet_type_info.impls_constraints[i].specific_id.index); new_facet_type_info.impls_constraints[i].specific_id = - SemIR::SpecificId(specific_id); + pop_specific(old_facet_type_info.impls_constraints[i].specific_id); } new_facet_type_info.Canonicalize(); return context.facet_types().Add(new_facet_type_info).index; diff --git a/toolchain/sem_ir/formatter.cpp b/toolchain/sem_ir/formatter.cpp index 6ee487912caf..48e136932bc0 100644 --- a/toolchain/sem_ir/formatter.cpp +++ b/toolchain/sem_ir/formatter.cpp @@ -20,6 +20,11 @@ #include "toolchain/sem_ir/name_scope.h" #include "toolchain/sem_ir/typed_insts.h" +// TODO: Consider addressing recursion here, although it's not critical because +// the formatter isn't required to work on arbitrary code. Still, it may help +// in the future to debug complex code. +// NOLINTBEGIN(misc-no-recursion) + namespace Carbon::SemIR { // Formatter for printing textual Semantics IR. @@ -1383,3 +1388,5 @@ auto Formatter::Print(llvm::raw_ostream& out) -> void { } } // namespace Carbon::SemIR + +// NOLINTEND(misc-no-recursion) diff --git a/toolchain/sem_ir/inst_namer.cpp b/toolchain/sem_ir/inst_namer.cpp index a33e2b57add1..b7a0f81c5b88 100644 --- a/toolchain/sem_ir/inst_namer.cpp +++ b/toolchain/sem_ir/inst_namer.cpp @@ -355,6 +355,9 @@ auto InstNamer::AddBlockLabel(ScopeId scope_id, SemIR::LocId loc_id, AddBlockLabel(scope_id, branch.target_id, name.str(), loc_id); } +// TODO: Consider addressing recursion here. It may be important because +// InstNamer is used for debug info. +// NOLINTNEXTLINE(misc-no-recursion) auto InstNamer::CollectNamesInBlock(ScopeId scope_id, InstBlockId block_id) -> void { if (block_id.is_valid()) { @@ -362,6 +365,7 @@ auto InstNamer::CollectNamesInBlock(ScopeId scope_id, InstBlockId block_id) } } +// NOLINTNEXTLINE(misc-no-recursion): See above TODO. auto InstNamer::CollectNamesInBlock(ScopeId scope_id, llvm::ArrayRef block) -> void { Scope& scope = GetScopeInfo(scope_id); diff --git a/toolchain/testing/yaml_test_helpers.cpp b/toolchain/testing/yaml_test_helpers.cpp index 4eff3f2a19e2..8bb9b877395c 100644 --- a/toolchain/testing/yaml_test_helpers.cpp +++ b/toolchain/testing/yaml_test_helpers.cpp @@ -9,6 +9,8 @@ namespace Carbon::Testing::Yaml { +// This is for tests, so we should be okay with the recursion here. +// NOLINTNEXTLINE(misc-no-recursion) static auto Parse(llvm::yaml::Node* node) -> Value { CARBON_CHECK(node != nullptr);