From c3da338ce4b23830ea75a21f66c18783440c80ef Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 22 Aug 2023 14:19:46 -0700 Subject: [PATCH] Make more use of llvm::zip and enumerate in explorer (#3135) I was initially looking at llvm::seq, but then looking through uses it seemed like enumerate/zip would be best for these, allowing less direct indexing and more range-based looping. --- explorer/interpreter/interpreter.cpp | 35 ++++++++-------- explorer/interpreter/interpreter.h | 9 ----- explorer/interpreter/type_checker.cpp | 58 ++++++++++++--------------- 3 files changed, 41 insertions(+), 61 deletions(-) diff --git a/explorer/interpreter/interpreter.cpp b/explorer/interpreter/interpreter.cpp index e967b8d27fb8..0b1ecd5c62a8 100644 --- a/explorer/interpreter/interpreter.cpp +++ b/explorer/interpreter/interpreter.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include "common/check.h" @@ -28,13 +27,14 @@ #include "explorer/base/trace_stream.h" #include "explorer/interpreter/action.h" #include "explorer/interpreter/action_stack.h" +#include "explorer/interpreter/heap.h" #include "explorer/interpreter/pattern_match.h" -#include "explorer/interpreter/stack.h" #include "explorer/interpreter/type_utils.h" #include "llvm/ADT/APInt.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Casting.h" -#include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/raw_ostream.h" @@ -290,10 +290,9 @@ auto Interpreter::EvalPrim(Operator op, Nonnull /*static_type*/, auto Interpreter::CreateStruct(const std::vector& fields, const std::vector>& values) -> Nonnull { - CARBON_CHECK(fields.size() == values.size()); std::vector elements; - for (size_t i = 0; i < fields.size(); ++i) { - elements.push_back({fields[i].name(), values[i]}); + for (const auto [field, value] : llvm::zip_equal(fields, values)) { + elements.push_back({field.name(), value}); } return arena_->New(std::move(elements)); @@ -741,14 +740,11 @@ auto Interpreter::Convert(Nonnull value, return value; } } - CARBON_CHECK(tuple->elements().size() == - destination_element_types.size()); std::vector> new_elements; - for (size_t i = 0; i < tuple->elements().size(); ++i) { - CARBON_ASSIGN_OR_RETURN( - Nonnull val, - Convert(tuple->elements()[i], destination_element_types[i], - source_loc)); + for (const auto [element, dest_type] : + llvm::zip_equal(tuple->elements(), destination_element_types)) { + CARBON_ASSIGN_OR_RETURN(Nonnull val, + Convert(element, dest_type, source_loc)); new_elements.push_back(val); } return arena_->New(std::move(new_elements)); @@ -1534,17 +1530,18 @@ auto Interpreter::StepExp() -> ErrorOr { // -> { {C',E',F'} :: {C, E, F} :: S, H} // Prepare parameters tuple. std::vector> param_values; - for (int i = 1; i <= num_args; ++i) { - param_values.push_back(act.results()[i]); + for (const auto& arg_result : + llvm::ArrayRef(act.results()).slice(1, num_args)) { + param_values.push_back(arg_result); } const auto* param_tuple = arena_->New(param_values); // Prepare witnesses. ImplWitnessMap witnesses; if (num_witnesses > 0) { - int i = 1 + num_args; - for (const auto& [impl_bind, impl_exp] : call.witnesses()) { - witnesses[impl_bind] = act.results()[i]; - ++i; + for (const auto [witness, result] : llvm::zip( + call.witnesses(), + llvm::ArrayRef(act.results()).drop_front(1 + num_args))) { + witnesses[witness.first] = result; } } return CallFunction(call, act.results()[0], param_tuple, diff --git a/explorer/interpreter/interpreter.h b/explorer/interpreter/interpreter.h index 3b5f4f959627..087a1f51aed0 100644 --- a/explorer/interpreter/interpreter.h +++ b/explorer/interpreter/interpreter.h @@ -5,20 +5,11 @@ #ifndef CARBON_EXPLORER_INTERPRETER_INTERPRETER_H_ #define CARBON_EXPLORER_INTERPRETER_INTERPRETER_H_ -#include -#include -#include - #include "common/ostream.h" #include "explorer/ast/ast.h" -#include "explorer/ast/declaration.h" #include "explorer/ast/expression.h" -#include "explorer/ast/pattern.h" #include "explorer/ast/value.h" #include "explorer/base/trace_stream.h" -#include "explorer/interpreter/action.h" -#include "explorer/interpreter/heap.h" -#include "llvm/ADT/ArrayRef.h" namespace Carbon { diff --git a/explorer/interpreter/type_checker.cpp b/explorer/interpreter/type_checker.cpp index c5fa1f4912a7..ac77dd40423a 100644 --- a/explorer/interpreter/type_checker.cpp +++ b/explorer/interpreter/type_checker.cpp @@ -5,7 +5,6 @@ #include "explorer/interpreter/type_checker.h" #include -#include #include #include #include @@ -32,11 +31,11 @@ #include "explorer/interpreter/pattern_match.h" #include "explorer/interpreter/type_structure.h" #include "explorer/interpreter/type_utils.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/TinyPtrVector.h" #include "llvm/Support/Casting.h" -#include "llvm/Support/Error.h" #include "llvm/Support/SaveAndRestore.h" using llvm::cast; @@ -469,11 +468,11 @@ auto TypeChecker::IsBuiltinConversion(SourceLocation source_loc, break; } bool all_ok = true; - for (size_t i = 0; i < source_tuple.elements().size(); ++i) { + for (const auto [source_elem, dest_elem] : llvm::zip_equal( + source_tuple.elements(), destination_tuple.elements())) { CARBON_ASSIGN_OR_RETURN( bool convertible, - IsImplicitlyConvertible(source_loc, source_tuple.elements()[i], - destination_tuple.elements()[i], + IsImplicitlyConvertible(source_loc, source_elem, dest_elem, impl_scope, allow_user_defined_conversions)); if (!convertible) { @@ -680,7 +679,8 @@ auto TypeChecker::BuildBuiltinConversion(Nonnull source, return conversion_failed(); } std::vector> converted_elements; - for (size_t i = 0; i < source_tuple.elements().size(); ++i) { + for (const auto [i, dest_elem] : + llvm::enumerate(destination_tuple.elements())) { auto* elem = arena_->New( source->source_loc(), source, arena_->New(source->source_loc(), i)); @@ -688,7 +688,7 @@ auto TypeChecker::BuildBuiltinConversion(Nonnull source, CARBON_ASSIGN_OR_RETURN( Nonnull converted, ImplicitlyConvert("implicit conversion", impl_scope, elem, - destination_tuple.elements()[i])); + dest_elem)); converted_elements.push_back(converted); } auto* result = arena_->New( @@ -853,12 +853,12 @@ auto TypeChecker::ImplicitlyConvert(std::string_view context, << "`" << *destination << "` of different length"; } std::vector> converted_elements; - for (size_t i = 0; i < source_tuple->fields().size(); ++i) { + for (const auto [source_field, dest_elem] : llvm::zip_equal( + source_tuple->fields(), destination_tuple->elements())) { CARBON_ASSIGN_OR_RETURN( Nonnull converted, - ImplicitlyConvert("implicit conversion", impl_scope, - source_tuple->fields()[i], - destination_tuple->elements()[i])); + ImplicitlyConvert("implicit conversion", impl_scope, source_field, + dest_elem)); converted_elements.push_back(converted); } auto* result = arena_->New(source->source_loc(), @@ -1163,10 +1163,10 @@ auto TypeChecker::ArgumentDeduction::Deduce(Nonnull param, << param_tup.elements().size() << " but got " << arg_tup.elements().size(); } - for (size_t i = 0; i < param_tup.elements().size(); ++i) { - CARBON_RETURN_IF_ERROR(Deduce(param_tup.elements()[i], - arg_tup.elements()[i], - allow_implicit_conversion)); + for (const auto [param_elem, arg_elem] : + llvm::zip_equal(param_tup.elements(), arg_tup.elements())) { + CARBON_RETURN_IF_ERROR( + Deduce(param_elem, arg_elem, allow_implicit_conversion)); } return Success(); } @@ -1207,10 +1207,8 @@ auto TypeChecker::ArgumentDeduction::Deduce(Nonnull param, << "duplicate field name?"; } } else { - size_t smaller_size = std::min(param_fields.size(), arg_fields.size()); - for (size_t i = 0; i < smaller_size; ++i) { - NamedValue param_field = param_fields[i]; - NamedValue arg_field = arg_fields[i]; + for (const auto [param_field, arg_field] : + llvm::zip(param_fields, arg_fields)) { if (param_field.name != arg_field.name) { return ProgramError(source_loc_) << "mismatch in field names, `" << param_field.name @@ -1592,8 +1590,7 @@ class TypeChecker::ConstraintTypeBuilder { // Adds an `impls` constraint -- `T impls C` if not already present. // Returns the index of the impls constraint within the self witness. auto AddImplsConstraint(ImplsConstraint impls) -> int { - for (int i = 0; i != static_cast(impls_constraints_.size()); ++i) { - ImplsConstraint& existing = impls_constraints_[i]; + for (const auto [i, existing] : llvm::enumerate(impls_constraints_)) { if (TypeEqual(existing.type, impls.type, std::nullopt) && TypeEqual(existing.interface, impls.interface, std::nullopt)) { return i; @@ -2508,9 +2505,7 @@ auto TypeChecker::DeduceCallBindings( // Deduce and/or convert each argument to the corresponding // parameter. - for (size_t i = 0; i < params.size(); ++i) { - const Value* param = params[i]; - Expression* arg = args[i]; + for (const auto [i, param, arg] : llvm::enumerate(params, args)) { if (!generic_params.empty() && generic_params.front().index == i) { // The parameter is a `:!` binding. Collect its argument so we can // evaluate it when we're done with deduction. @@ -3658,11 +3653,10 @@ auto TypeChecker::TypeCheckExpImpl(Nonnull e, // Collect the top-level generic parameters and their constraints. std::vector generic_parameters; - llvm::ArrayRef> params = - param_name.params().fields(); - for (size_t i = 0; i != params.size(); ++i) { + for (const auto [i, param] : + llvm::enumerate(param_name.params().fields())) { // TODO: Should we disallow all other kinds of top-level params? - if (const auto* binding = dyn_cast(params[i])) { + if (const auto* binding = dyn_cast(param)) { generic_parameters.push_back({{}, i, binding}); } } @@ -4390,8 +4384,7 @@ auto TypeChecker::TypeCheckPattern( cast(**expected).elements().size()) { return ProgramError(tuple.source_loc()) << "tuples of different length"; } - for (size_t i = 0; i < tuple.fields().size(); ++i) { - Nonnull field = tuple.fields()[i]; + for (const auto [i, field] : llvm::enumerate(tuple.fields())) { std::optional> expected_field_type; if (expected) { expected_field_type = cast(**expected).elements()[i]; @@ -4946,9 +4939,8 @@ auto TypeChecker::DeclareCallableDeclaration(Nonnull f, // Keep track of any generic parameters and nested generic bindings in the // parameter pattern. std::vector generic_parameters; - for (size_t i = 0; i != f->param_pattern().fields().size(); ++i) { - Pattern* param_pattern = f->param_pattern().fields()[i]; - + for (const auto [i, param_pattern] : + llvm::enumerate(f->param_pattern().fields())) { size_t old_size = all_bindings.size(); CollectAndNumberGenericBindingsInPattern(param_pattern, all_bindings);