Przeglądaj źródła

Cleanup pass over llvm::seq uses (#5185)

I was thinking about this after `seq` changes in #5182, and looked for
other uses that might be replaceable. Here's the resulting cleanup
around `seq`:

- Switch to `enumerate` or `zip` when possible.
- `int _` -> `auto _` (it's typically a `size_t`, but there's no reason
to cast when unused)
- Fix a case of cast style `(size_t)...` -> `static_cast<size_t>(...)`
- Switch `(void)close_children_count` to `[[maybe_unused]]`
Jon Ross-Perkins 1 rok temu
rodzic
commit
0d3d829478

+ 3 - 5
common/command_line.cpp

@@ -505,10 +505,9 @@ auto MetaPrinter::PrintArgLongValues(const Arg& arg,
   *out_ << indent << "Possible values:\n";
   // TODO: It would be good to add help text for each value and then print it
   // here.
-  for (int i : llvm::seq<int>(0, arg.value_strings.size())) {
-    llvm::StringRef value_string = arg.value_strings[i];
+  for (auto [i, value_string] : llvm::enumerate(arg.value_strings)) {
     *out_ << indent << "- " << value_string;
-    if (arg.has_default && i == arg.default_value_index) {
+    if (arg.has_default && static_cast<int>(i) == arg.default_value_index) {
       *out_ << " (default)";
     }
     *out_ << "\n";
@@ -619,12 +618,11 @@ auto MetaPrinter::PrintRawUsage(const Command& command,
 
     if (!command.positional_args.empty()) {
       bool open_optional = false;
-      for (int i : llvm::seq<int>(0, command.positional_args.size())) {
+      for (auto [i, arg] : llvm::enumerate(command.positional_args)) {
         *out_ << " ";
         if (i != 0 && command.positional_args[i - 1]->is_append) {
           *out_ << "-- ";
         }
-        const auto& arg = command.positional_args[i];
         if (!arg->is_required && !open_optional) {
           *out_ << "[";
           open_optional = true;

+ 4 - 3
common/command_line.h

@@ -812,9 +812,10 @@ auto OneOfArgBuilder::OneOfImpl(const OneOfValueT<U> (&input_values)[N],
   // by index.
   new (&arg()->value_action) Arg::ValueActionT(
       [values, match](const Arg& arg, llvm::StringRef value_string) -> bool {
-        for (int i : llvm::seq<int>(0, N)) {
-          if (value_string == arg.value_strings[i]) {
-            match(values[i]);
+        for (auto [value, arg_value_string] :
+             llvm::zip(values, arg.value_strings)) {
+          if (value_string == arg_value_string) {
+            match(value);
             return true;
           }
         }

+ 8 - 8
testing/base/source_gen.cpp

@@ -347,7 +347,7 @@ auto SourceGen::GenApiFileDenseDecls(int target_lines,
 
   auto class_gen_state = ClassGenState(*this, num_classes, params.class_params,
                                        params.type_use_params);
-  for ([[maybe_unused]] int _ : llvm::seq(num_classes)) {
+  for ([[maybe_unused]] auto _ : llvm::seq(num_classes)) {
     source << "\n";
     GenerateClassDef(params.class_params, class_gen_state, source);
   }
@@ -421,7 +421,7 @@ auto SourceGen::GetSingleLengthIdentifiers(int length, int number)
 
   if (static_cast<int>(idents.size()) < number) {
     idents.reserve(number);
-    for ([[maybe_unused]] int _ : llvm::seq<int>(idents.size(), number)) {
+    for ([[maybe_unused]] auto _ : llvm::seq<int>(idents.size(), number)) {
       auto ident_storage =
           llvm::MutableArrayRef(reinterpret_cast<char*>(storage_.Allocate(
                                     /*Size=*/length, /*Alignment=*/1)),
@@ -523,7 +523,7 @@ auto SourceGen::AppendUniqueIdentifiers(
     unique_idents.GrowForInsertCount(count - number);
 
     // Generate the needed number of identifiers.
-    for ([[maybe_unused]] int _ : llvm::seq<int>(count, number)) {
+    for ([[maybe_unused]] auto _ : llvm::seq<int>(count, number)) {
       // Allocate stable storage for the identifier so we can form stable
       // `StringRef`s to it.
       auto ident_storage =
@@ -849,7 +849,7 @@ auto SourceGen::GenerateClassDef(const ClassParams& params,
   // by collecting them before inserting that type into the valid set.
   llvm::SmallVector<llvm::StringRef> field_type_names;
   field_type_names.reserve(params.private_field_decls);
-  for ([[maybe_unused]] int _ : llvm::seq(params.private_field_decls)) {
+  for ([[maybe_unused]] auto _ : llvm::seq(params.private_field_decls)) {
     field_type_names.push_back(state.GetValidTypeName());
   }
 
@@ -860,7 +860,7 @@ auto SourceGen::GenerateClassDef(const ClassParams& params,
 
   UniqueIdentifierPopper unique_member_names(*this, state.member_names());
   llvm::ListSeparator line_sep("\n");
-  for ([[maybe_unused]] int _ : llvm::seq(params.public_function_decls)) {
+  for ([[maybe_unused]] auto _ : llvm::seq(params.public_function_decls)) {
     os << line_sep;
     GenerateFunctionDecl(
         unique_member_names.Pop(), /*is_private=*/false,
@@ -869,7 +869,7 @@ auto SourceGen::GenerateClassDef(const ClassParams& params,
         /*indent=*/"  ", state.param_names(),
         [&] { return state.GetValidTypeName(); }, os);
   }
-  for ([[maybe_unused]] int _ : llvm::seq(params.public_method_decls)) {
+  for ([[maybe_unused]] auto _ : llvm::seq(params.public_method_decls)) {
     os << line_sep;
     GenerateFunctionDecl(
         unique_member_names.Pop(), /*is_private=*/false,
@@ -884,7 +884,7 @@ auto SourceGen::GenerateClassDef(const ClassParams& params,
     line_sep = llvm::ListSeparator("\n");
   }
 
-  for ([[maybe_unused]] int _ : llvm::seq(params.private_function_decls)) {
+  for ([[maybe_unused]] auto _ : llvm::seq(params.private_function_decls)) {
     os << line_sep;
     GenerateFunctionDecl(
         unique_member_names.Pop(), /*is_private=*/true,
@@ -893,7 +893,7 @@ auto SourceGen::GenerateClassDef(const ClassParams& params,
         /*indent=*/"  ", state.param_names(),
         [&] { return state.GetValidTypeName(); }, os);
   }
-  for ([[maybe_unused]] int _ : llvm::seq(params.private_method_decls)) {
+  for ([[maybe_unused]] auto _ : llvm::seq(params.private_method_decls)) {
     os << line_sep;
     GenerateFunctionDecl(
         unique_member_names.Pop(), /*is_private=*/true,

+ 1 - 1
testing/base/source_gen_test.cpp

@@ -70,7 +70,7 @@ TEST(SourceGenTest, Identifiers) {
   // Check that repeated calls are different in interesting ways, but have the
   // exact same total bytes.
   ssize_t idents_size_sum = SumSizes(idents);
-  for ([[maybe_unused]] int _ : llvm::seq(10)) {
+  for ([[maybe_unused]] auto _ : llvm::seq(10)) {
     auto idents2 = gen.GetShuffledIdentifiers(1000);
     EXPECT_THAT(idents2, SizeIs(1000));
     // Should be (at least) a different shuffle of identifiers.

+ 4 - 4
toolchain/check/impl.cpp

@@ -117,13 +117,13 @@ auto ImplWitnessStartDefinition(Context& context, SemIR::Impl& impl) -> void {
 
   // Check we have a value for all non-function associated constants in the
   // witness.
-  for (auto index : llvm::seq(assoc_entities.size())) {
-    auto decl_id = assoc_entities[index];
+  for (auto [assoc_entity, witness_value] :
+       llvm::zip(assoc_entities, witness_block)) {
+    auto decl_id = assoc_entity;
     decl_id = context.constant_values().GetConstantInstId(decl_id);
     CARBON_CHECK(decl_id.has_value(), "Non-constant associated entity");
     if (auto decl =
             context.insts().TryGetAs<SemIR::AssociatedConstantDecl>(decl_id)) {
-      auto& witness_value = witness_block[index];
       if (!witness_value.has_value()) {
         CARBON_DIAGNOSTIC(ImplAssociatedConstantNeedsValue, Error,
                           "associated constant {0} not given a value in impl "
@@ -137,7 +137,7 @@ auto ImplWitnessStartDefinition(Context& context, SemIR::Impl& impl) -> void {
                        .Get(decl->assoc_const_id)
                        .name_id,
                    interface.name_id)
-            .Note(assoc_entities[index], AssociatedConstantHere)
+            .Note(assoc_entity, AssociatedConstantHere)
             .Emit();
 
         witness_value = SemIR::ErrorInst::SingletonInstId;

+ 9 - 10
toolchain/check/subst.cpp

@@ -230,22 +230,21 @@ static auto PopOperand(Context& context, Worklist& worklist, SemIR::IdKind kind,
       new_facet_type_info.rewrite_constraints.resize(
           old_facet_type_info.rewrite_constraints.size(),
           SemIR::FacetTypeInfo::RewriteConstraint::None);
-      for (auto i : llvm::reverse(
-               llvm::seq(old_facet_type_info.rewrite_constraints.size()))) {
+      for (auto& new_constraint :
+           llvm::reverse(new_facet_type_info.rewrite_constraints)) {
         auto rhs_id = context.constant_values().Get(worklist.Pop());
         auto lhs_id = context.constant_values().Get(worklist.Pop());
-        new_facet_type_info.rewrite_constraints[i] = {.lhs_const_id = lhs_id,
-                                                      .rhs_const_id = rhs_id};
+        new_constraint = {.lhs_const_id = lhs_id, .rhs_const_id = rhs_id};
       }
       new_facet_type_info.impls_constraints.resize(
           old_facet_type_info.impls_constraints.size(),
           SemIR::SpecificInterface::None);
-      for (auto i : llvm::reverse(
-               llvm::seq(old_facet_type_info.impls_constraints.size()))) {
-        const auto& old = old_facet_type_info.impls_constraints[i];
-        new_facet_type_info.impls_constraints[i] = {
-            .interface_id = old.interface_id,
-            .specific_id = pop_specific(old.specific_id)};
+      for (auto [old_constraint, new_constraint] :
+           llvm::reverse(llvm::zip(old_facet_type_info.impls_constraints,
+                                   new_facet_type_info.impls_constraints))) {
+        new_constraint = {
+            .interface_id = old_constraint.interface_id,
+            .specific_id = pop_specific(old_constraint.specific_id)};
       }
       new_facet_type_info.other_requirements =
           old_facet_type_info.other_requirements;

+ 4 - 4
toolchain/driver/compile_benchmark.cpp

@@ -34,10 +34,10 @@ class CompileBenchmark {
   auto SetUpFiles(llvm::ArrayRef<std::string> sources)
       -> llvm::OwningArrayRef<std::string> {
     llvm::OwningArrayRef<std::string> file_names(sources.size());
-    for (ssize_t i : llvm::seq<ssize_t>(sources.size())) {
-      file_names[i] = llvm::formatv("file_{0}.carbon", i).str();
-      fs_->addFile(file_names[i], /*ModificationTime=*/0,
-                   llvm::MemoryBuffer::getMemBuffer(sources[i]));
+    for (auto [i, source, file_name] : llvm::enumerate(sources, file_names)) {
+      file_name = llvm::formatv("file_{0}.carbon", i).str();
+      fs_->addFile(file_name, /*ModificationTime=*/0,
+                   llvm::MemoryBuffer::getMemBuffer(source));
     }
     return file_names;
   }

+ 2 - 2
toolchain/lex/tokenized_buffer_test.cpp

@@ -1034,7 +1034,7 @@ TEST_F(LexerTest, TypeLiteralTooManyDigits) {
   // A 128-bit APInt should be plenty large, but if needed in the future it can
   // be widened without issue.
   llvm::APInt bits = llvm::APInt::getZero(128);
-  for ([[maybe_unused]] int _ : llvm::seq(1, 30)) {
+  for ([[maybe_unused]] auto _ : llvm::seq(1, 30)) {
     code.append("9");
     bits = bits * 10 + 9;
     auto [buffer, value_stores] =
@@ -1160,7 +1160,7 @@ TEST_F(LexerTest, DiagnosticFileTooLarge) {
   static constexpr size_t NumLines = 10'000'000;
   std::string input;
   input.reserve(NumLines * 3);
-  for ([[maybe_unused]] int _ : llvm::seq(NumLines)) {
+  for ([[maybe_unused]] auto _ : llvm::seq(NumLines)) {
     input += "{}\n";
   }
   EXPECT_CALL(consumer,

+ 2 - 3
toolchain/parse/tree_and_subtrees.cpp

@@ -27,7 +27,7 @@ TreeAndSubtrees::TreeAndSubtrees(const Lex::TokenizedBuffer& tokens,
           kind, size_stack.size());
       for (auto i : llvm::seq(kind.child_count())) {
         auto child = size_stack.pop_back_val();
-        CARBON_CHECK((size_t)child.index < subtree_sizes_.size());
+        CARBON_CHECK(static_cast<size_t>(child.index) < subtree_sizes_.size());
         size += subtree_sizes_[child.index];
         if (kind.has_bracket() && i == kind.child_count() - 1) {
           CARBON_CHECK(kind.bracket() == tree.node_kind(child),
@@ -223,8 +223,7 @@ auto TreeAndSubtrees::PrintPreorder(llvm::raw_ostream& output) const -> void {
 
     int next_depth = node_stack.empty() ? 0 : node_stack.back().second;
     CARBON_CHECK(next_depth <= depth, "Cannot have the next depth increase!");
-    for (int close_children_count : llvm::seq(0, depth - next_depth)) {
-      (void)close_children_count;
+    for ([[maybe_unused]] auto _ : llvm::seq(depth - next_depth)) {
       output << "]}";
     }