From fbc7690157e3d6076824d1e284079d61db797416 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Mon, 17 Nov 2025 16:28:06 -0800 Subject: [PATCH] Switch `zip` to `zip_equal` where possible (#6389) There are two uses I'm not converting here, that seem to want the "shortest" behavior. For everything else, I'm going to `zip_equal` since it's more restrictive. I wish `zip` were named `zip_shortest`. --- common/command_line.h | 2 +- toolchain/check/eval.cpp | 6 +++--- toolchain/check/handle_struct.cpp | 3 ++- toolchain/check/impl.cpp | 4 ++-- toolchain/check/impl_lookup.cpp | 2 +- toolchain/check/import_ref.cpp | 8 ++++---- toolchain/check/pattern_match.cpp | 2 +- toolchain/check/subst.cpp | 21 +++++++++++---------- toolchain/driver/runtimes_cache_test.cpp | 2 +- toolchain/lower/specific_coalescer.cpp | 2 +- toolchain/parse/tree_and_subtrees.cpp | 5 +++-- toolchain/sem_ir/formatter.cpp | 6 +++--- toolchain/sem_ir/stringify.cpp | 2 +- 13 files changed, 34 insertions(+), 31 deletions(-) diff --git a/common/command_line.h b/common/command_line.h index 3eb64dd1215a..e7fc73f1cd5e 100644 --- a/common/command_line.h +++ b/common/command_line.h @@ -813,7 +813,7 @@ auto OneOfArgBuilder::OneOfImpl(const OneOfValueT (&input_values)[N], new (&arg()->value_action) Arg::ValueActionT( [values, match](const Arg& arg, llvm::StringRef value_string) -> bool { for (auto [value, arg_value_string] : - llvm::zip(values, arg.value_strings)) { + llvm::zip_equal(values, arg.value_strings)) { if (value_string == arg_value_string) { match(value); return true; diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index 5dc529f51cf9..3bffbefa4e6a 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -1720,9 +1720,9 @@ static auto MakeConstantForBuiltinCall(EvalContext& eval_context, CARBON_CHECK(arg_ids.size() == 2); auto lhs_facet_type_id = SemIR::FacetTypeId::None; auto rhs_facet_type_id = SemIR::FacetTypeId::None; - for (auto [facet_type_id, type_arg_id] : - llvm::zip(std::to_array({&lhs_facet_type_id, &rhs_facet_type_id}), - context.types().GetBlockAsTypeInstIds(arg_ids))) { + for (auto [facet_type_id, type_arg_id] : llvm::zip_equal( + std::to_array({&lhs_facet_type_id, &rhs_facet_type_id}), + context.types().GetBlockAsTypeInstIds(arg_ids))) { if (auto facet_type = context.insts().TryGetAs(type_arg_id)) { *facet_type_id = facet_type->facet_type_id; diff --git a/toolchain/check/handle_struct.cpp b/toolchain/check/handle_struct.cpp index a2320c311f90..b2d5978a0e3c 100644 --- a/toolchain/check/handle_struct.cpp +++ b/toolchain/check/handle_struct.cpp @@ -82,7 +82,8 @@ static auto DiagnoseDuplicateNames( llvm::ArrayRef fields, bool is_struct_type_literal) -> bool { Map names; - for (auto [field_name_node, field] : llvm::zip(field_name_nodes, fields)) { + for (auto [field_name_node, field] : + llvm::zip_equal(field_name_nodes, fields)) { auto result = names.Insert(field.name_id, field_name_node); if (!result.is_inserted()) { CARBON_DIAGNOSTIC(StructNameDuplicate, Error, diff --git a/toolchain/check/impl.cpp b/toolchain/check/impl.cpp index 86ec08306325..9a529b4a5c5c 100644 --- a/toolchain/check/impl.cpp +++ b/toolchain/check/impl.cpp @@ -137,7 +137,7 @@ auto ImplWitnessStartDefinition(Context& context, SemIR::Impl& impl) -> void { // Check we have a value for all non-function associated constants in the // witness. for (auto [assoc_entity, witness_value] : - llvm::zip(assoc_entities, witness_block)) { + llvm::zip_equal(assoc_entities, witness_block)) { auto decl_id = context.constant_values().GetConstantInstId(assoc_entity); CARBON_CHECK(decl_id.has_value(), "Non-constant associated entity"); if (auto decl = @@ -187,7 +187,7 @@ auto FinishImplWitness(Context& context, SemIR::ImplId impl_id) -> void { llvm::SmallVector used_decl_ids; for (auto [assoc_entity, witness_value] : - llvm::zip(assoc_entities, witness_block)) { + llvm::zip_equal(assoc_entities, witness_block)) { auto decl_id = context.constant_values().GetInstId(SemIR::GetConstantValueInSpecific( context.sem_ir(), impl.interface.specific_id, assoc_entity)); diff --git a/toolchain/check/impl_lookup.cpp b/toolchain/check/impl_lookup.cpp index 4482f2708974..d1398828271d 100644 --- a/toolchain/check/impl_lookup.cpp +++ b/toolchain/check/impl_lookup.cpp @@ -459,7 +459,7 @@ class SubstWitnessesCallbacks : public SubstInstCallbacks { auto lookup_query_interface = context().specific_interfaces().Get(specific_interface_id); for (auto [interface, witness_inst_id] : - llvm::zip(interfaces_, witness_inst_ids_)) { + llvm::zip_equal(interfaces_, witness_inst_ids_)) { // If the `LookupImplWitness` for `.Self` is not looking for the same // interface as we have a witness for, this is not the right witness to // use to replace the lookup for `.Self`. diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index a3c927299241..92d9b6aa002b 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -2157,7 +2157,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, } for (auto [import_vtable_entry_inst_id, local_vtable_entry_inst_id] : - llvm::zip(virtual_functions, lazy_virtual_functions)) { + llvm::zip_equal(virtual_functions, lazy_virtual_functions)) { // Use LoadedImportRef for imported symbolic constant vtable entries so they // can carry attached constants necessary for applying specifics to these // constants when they are used. @@ -3211,7 +3211,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, llvm::SmallVector new_fields; new_fields.reserve(orig_fields.size()); for (auto [orig_field, field_type_inst_id] : - llvm::zip(orig_fields, field_type_inst_ids)) { + llvm::zip_equal(orig_fields, field_type_inst_ids)) { auto name_id = GetLocalNameId(resolver, orig_field.name_id); new_fields.push_back( {.name_id = name_id, .type_inst_id = field_type_inst_id}); @@ -3909,8 +3909,8 @@ static auto ResolveLocalEvalBlock(ImportRefResolver& resolver, // Set the locations of the instructions in the inst block to match those of // the imported instructions. for (auto [import_inst_id, local_inst_id] : - llvm::zip(resolver.import_inst_blocks().Get(import_block_id), - resolver.local_inst_blocks().Get(eval_block_id))) { + llvm::zip_equal(resolver.import_inst_blocks().Get(import_block_id), + resolver.local_inst_blocks().Get(eval_block_id))) { auto import_ir_inst_id = AddImportIRInst(resolver, import_inst_id); resolver.local_insts().SetLocId(local_inst_id, import_ir_inst_id); } diff --git a/toolchain/check/pattern_match.cpp b/toolchain/check/pattern_match.cpp index 91c81d4ec094..4cd785101b36 100644 --- a/toolchain/check/pattern_match.cpp +++ b/toolchain/check/pattern_match.cpp @@ -527,7 +527,7 @@ auto MatchContext::DoEmitPatternMatch(Context& context, auto add_all_subscrutinees = [&](llvm::ArrayRef subscrutinee_ids) { for (auto [subpattern_id, subscrutinee_id] : - llvm::reverse(llvm::zip(subpattern_ids, subscrutinee_ids))) { + llvm::reverse(llvm::zip_equal(subpattern_ids, subscrutinee_ids))) { AddWork( {.pattern_id = subpattern_id, .scrutinee_id = subscrutinee_id}); } diff --git a/toolchain/check/subst.cpp b/toolchain/check/subst.cpp index 0fb214c43f2b..37ebba0f7e78 100644 --- a/toolchain/check/subst.cpp +++ b/toolchain/check/subst.cpp @@ -252,9 +252,10 @@ static auto PopOperand(Context& context, Worklist& worklist, new_facet_type_info.self_impls_named_constraints.resize( old_facet_type_info.self_impls_named_constraints.size(), SemIR::SpecificNamedConstraint::None); - for (auto [old_constraint, new_constraint] : llvm::reverse( - llvm::zip(old_facet_type_info.self_impls_named_constraints, - new_facet_type_info.self_impls_named_constraints))) { + for (auto [old_constraint, new_constraint] : + llvm::reverse(llvm::zip_equal( + old_facet_type_info.self_impls_named_constraints, + new_facet_type_info.self_impls_named_constraints))) { new_constraint = { .named_constraint_id = old_constraint.named_constraint_id, .specific_id = pop_specific(old_constraint.specific_id)}; @@ -263,8 +264,8 @@ static auto PopOperand(Context& context, Worklist& worklist, old_facet_type_info.extend_named_constraints.size(), SemIR::SpecificNamedConstraint::None); for (auto [old_constraint, new_constraint] : llvm::reverse( - llvm::zip(old_facet_type_info.extend_named_constraints, - new_facet_type_info.extend_named_constraints))) { + llvm::zip_equal(old_facet_type_info.extend_named_constraints, + new_facet_type_info.extend_named_constraints))) { new_constraint = { .named_constraint_id = old_constraint.named_constraint_id, .specific_id = pop_specific(old_constraint.specific_id)}; @@ -273,8 +274,8 @@ static auto PopOperand(Context& context, Worklist& worklist, old_facet_type_info.self_impls_constraints.size(), SemIR::SpecificInterface::None); for (auto [old_constraint, new_constraint] : llvm::reverse( - llvm::zip(old_facet_type_info.self_impls_constraints, - new_facet_type_info.self_impls_constraints))) { + llvm::zip_equal(old_facet_type_info.self_impls_constraints, + new_facet_type_info.self_impls_constraints))) { new_constraint = { .interface_id = old_constraint.interface_id, .specific_id = pop_specific(old_constraint.specific_id)}; @@ -282,9 +283,9 @@ static auto PopOperand(Context& context, Worklist& worklist, new_facet_type_info.extend_constraints.resize( old_facet_type_info.extend_constraints.size(), SemIR::SpecificInterface::None); - for (auto [old_constraint, new_constraint] : - llvm::reverse(llvm::zip(old_facet_type_info.extend_constraints, - new_facet_type_info.extend_constraints))) { + for (auto [old_constraint, new_constraint] : llvm::reverse( + llvm::zip_equal(old_facet_type_info.extend_constraints, + new_facet_type_info.extend_constraints))) { new_constraint = { .interface_id = old_constraint.interface_id, .specific_id = pop_specific(old_constraint.specific_id)}; diff --git a/toolchain/driver/runtimes_cache_test.cpp b/toolchain/driver/runtimes_cache_test.cpp index 29816cb71336..6b71feb7523d 100644 --- a/toolchain/driver/runtimes_cache_test.cpp +++ b/toolchain/driver/runtimes_cache_test.cpp @@ -251,7 +251,7 @@ TEST_F(RuntimesCacheTest, BasicBuild) { } for (const auto& [target, built_runtimes_path] : - llvm::zip(targets, built_runtimes_paths)) { + llvm::zip_equal(targets, built_runtimes_paths)) { SCOPED_TRACE(target); auto lookup_result = cache_.Lookup({.target = target}); ASSERT_THAT(lookup_result, IsSuccess(_)); diff --git a/toolchain/lower/specific_coalescer.cpp b/toolchain/lower/specific_coalescer.cpp index 4f862a6bfc53..24a15f9aaf08 100644 --- a/toolchain/lower/specific_coalescer.cpp +++ b/toolchain/lower/specific_coalescer.cpp @@ -228,7 +228,7 @@ auto SpecificCoalescer::AreFunctionBodiesEquivalent( "Number of specific calls expected to be the same."); for (auto [state1_call, state2_call] : - llvm::zip(state1.calls, state2.calls)) { + llvm::zip_equal(state1.calls, state2.calls)) { if (state1_call != state2_call) { if (ContainsPair(state1_call, state2_call, non_equivalent_specifics_)) { return false; diff --git a/toolchain/parse/tree_and_subtrees.cpp b/toolchain/parse/tree_and_subtrees.cpp index e55c230f216b..9993be4702d6 100644 --- a/toolchain/parse/tree_and_subtrees.cpp +++ b/toolchain/parse/tree_and_subtrees.cpp @@ -170,13 +170,14 @@ auto TreeAndSubtrees::Print(llvm::raw_ostream& output) const -> void { // Walk the tree in reverse, just to calculate depths for each node. llvm::SmallVector depths(tree_->size(), 0); - for (auto [n, depth] : llvm::reverse(llvm::zip(tree_->postorder(), depths))) { + for (auto [n, depth] : + llvm::reverse(llvm::zip_equal(tree_->postorder(), depths))) { for (auto child : children(n)) { depths[child.index] = depth + 1; } } - for (auto [n, depth] : llvm::zip(tree_->postorder(), depths)) { + for (auto [n, depth] : llvm::zip_equal(tree_->postorder(), depths)) { PrintNode(output, n, depth, /*preorder=*/false); output << ",\n"; } diff --git a/toolchain/sem_ir/formatter.cpp b/toolchain/sem_ir/formatter.cpp index 31939acf81ef..1b01a5fef9af 100644 --- a/toolchain/sem_ir/formatter.cpp +++ b/toolchain/sem_ir/formatter.cpp @@ -1090,9 +1090,9 @@ auto Formatter::FormatInstRhs(Inst inst) -> void { auto layout = sem_ir_->custom_layouts().Get(type.layout_id); out_ << "size=" << layout[CustomLayoutId::SizeIndex] << ", align=" << layout[CustomLayoutId::AlignIndex]; - for (auto [field, offset] : - llvm::zip(sem_ir_->struct_type_fields().Get(type.fields_id), - layout.drop_front(CustomLayoutId::FirstFieldIndex))) { + for (auto [field, offset] : llvm::zip_equal( + sem_ir_->struct_type_fields().Get(type.fields_id), + layout.drop_front(CustomLayoutId::FirstFieldIndex))) { out_ << ", ."; FormatName(field.name_id); out_ << "@" << offset << ": "; diff --git a/toolchain/sem_ir/stringify.cpp b/toolchain/sem_ir/stringify.cpp index 57b93db00efd..119ed5d39bc0 100644 --- a/toolchain/sem_ir/stringify.cpp +++ b/toolchain/sem_ir/stringify.cpp @@ -654,7 +654,7 @@ class Stringifier { step_stack_->PushString("}"); llvm::ListSeparator sep; for (auto [field, value_inst_id] : - llvm::reverse(llvm::zip(fields, field_values))) { + llvm::reverse(llvm::zip_equal(fields, field_values))) { step_stack_->Push(".", field.name_id, " = ", value_inst_id, &sep); } }