[DSLX:TS][cleanup] Remove unnecessary "order" in parametric instantiation.

The parametric order is now implied by the parametrics as declared, since
parametrics are evaluated from left to right.

This is part of the general effort to move away from string-oriented parametric
environments (as keys) and towards AST-node-oriented parametric environments.
(When we have fully migrated to AST-node-oriented parametric environments we
should be able to remove ParametricSymbols as ConcreteTypeDims.)

Historical note: previously we would attempt to do more of a "constraint
satisfaction" arbitrary ordering, but the value-to-confusion ratio of that
approach is not good, so we settled on this left-to-right evaluation.

PiperOrigin-RevId: 625088126

Author: cdleary

xls/dslx/type_system/parametric_instantiator_internal.cc

@@ -142,12 +142,14 @@ absl::StatusOr<InterpValue> InterpretExpr(DeduceCtx* ctx, Expr* expr,
 // asserting that their values are consistent with other bindings (via argument
 // types).
 absl::Status EagerlyPopulateParametricEnvMap(
-    absl::Span<const std::string> parametric_order,
+    absl::Span<const ParametricWithType> typed_parametrics,
     const absl::flat_hash_map<std::string, Expr*>& parametric_default_exprs,
     absl::flat_hash_map<std::string, InterpValue>& parametric_env_map,
     const Span& span, std::string_view kind_name, DeduceCtx* ctx) {
   // Attempt to interpret the parametric "default expressions" in order.
-  for (const auto& name : parametric_order) {
+  for (const ParametricWithType& typed_parametric : typed_parametrics) {
+    std::string_view name = typed_parametric.identifier();
+
     Expr* expr = nullptr;
     if (auto it = parametric_default_exprs.find(name);
         it != parametric_default_exprs.end() && it->second != nullptr) {
@@ -205,7 +207,7 @@ absl::Status EagerlyPopulateParametricEnvMap(
         return TypeInferenceErrorStatus(span, nullptr, message);
       }
     } else {
-      parametric_env_map.insert({name, result.value()});
+      parametric_env_map.insert({std::string{name}, result.value()});
     }
   }
   return absl::OkStatus();
@@ -222,6 +224,7 @@ ParametricInstantiator::ParametricInstantiator(
     : span_(std::move(span)),
       args_(args),
       ctx_(ABSL_DIE_IF_NULL(ctx)),
+      typed_parametrics_(typed_parametrics),
       parametric_env_map_(explicit_parametrics),
       parametric_bindings_(parametric_bindings) {
   // We add derived type information so we can resolve types based on
@@ -233,10 +236,6 @@ ParametricInstantiator::ParametricInstantiator(
   //  interpret the expression to an InterpValue.
   derived_type_info_ = ctx_->AddDerivedTypeInfo();
 
-  // Explicit parametric expressions are conceptually evaluated before other
-  // parametric expressions.
-  absl::flat_hash_set<std::string> ordered;
-
   // Note: the first N explicit parametrics (given by the map) must be the first
   // N parametrics for the thing we're instantiating; i.e. we can only
   // explicitly instantiate from the "left hand side" of the parametric
@@ -250,28 +249,21 @@ ParametricInstantiator::ParametricInstantiator(
   for (size_t i = 0; i < explicit_parametrics.size(); ++i) {
     const std::string& identifier = parametric_bindings[i]->identifier();
     CHECK(explicit_parametrics.contains(identifier));
-    parametric_order_.push_back(identifier);
-    ordered.insert(identifier);
   }
 
-  VLOG(5) << "ParametricInstantiator; span: " << span_ << " ordered: ["
-          << absl::StrJoin(ordered, ", ") << "]";
+  VLOG(5) << "ParametricInstantiator; span: " << span_;
 
   for (const ParametricWithType& parametric : typed_parametrics) {
-    std::string_view identifier = parametric.identifier();
-    if (!ordered.contains(identifier)) {
-      parametric_order_.push_back(std::string{identifier});
-      ordered.insert(std::string{identifier});
+    if (parametric.expr() != nullptr) {
+      ctx_->type_info()->SetItem(parametric.expr(), parametric.type());
     }
 
+    std::string_view identifier = parametric.identifier();
     std::unique_ptr<Type> parametric_expr_type =
         parametric.type().CloneToUnique();
-
-    if (parametric.expr() != nullptr) {
-      ctx_->type_info()->SetItem(parametric.expr(), *parametric_expr_type);
-    }
     parametric_binding_types_.emplace(identifier,
                                       std::move(parametric_expr_type));
+
     parametric_default_exprs_[identifier] = parametric.expr();
   }
 }
@@ -345,7 +337,7 @@ absl::StatusOr<TypeAndParametricEnv> FunctionInstantiator::Instantiate() {
   }
 
   XLS_RETURN_IF_ERROR(EagerlyPopulateParametricEnvMap(
-      parametric_order(), parametric_default_exprs(), parametric_env_map(),
+      typed_parametrics(), parametric_default_exprs(), parametric_env_map(),
       span(), GetKindName(), &ctx()));
 
   // Phase 2: resolve and check.
@@ -382,8 +374,9 @@ absl::StatusOr<TypeAndParametricEnv> FunctionInstantiator::Instantiate() {
 
   parametric_env_expr_scope.Finish();
 
-  return TypeAndParametricEnv{std::move(resolved),
-                              ParametricEnv(parametric_env_map())};
+  return TypeAndParametricEnv{
+      .type = std::move(resolved),
+      .parametric_env = ParametricEnv(parametric_env_map())};
 }
 
 /* static */ absl::StatusOr<std::unique_ptr<StructInstantiator>>
@@ -409,7 +402,7 @@ absl::StatusOr<TypeAndParametricEnv> StructInstantiator::Instantiate() {
   }
 
   XLS_RETURN_IF_ERROR(EagerlyPopulateParametricEnvMap(
-      parametric_order(), parametric_default_exprs(), parametric_env_map(),
+      typed_parametrics(), parametric_default_exprs(), parametric_env_map(),
       span(), GetKindName(), &ctx()));
 
   // Phase 2: resolve and check.
@@ -427,8 +420,9 @@ absl::StatusOr<TypeAndParametricEnv> StructInstantiator::Instantiate() {
 
   XLS_ASSIGN_OR_RETURN(std::unique_ptr<Type> resolved,
                        Resolve(*struct_type_, parametric_env_map()));
-  return TypeAndParametricEnv{std::move(resolved),
-                              ParametricEnv(parametric_env_map())};
+  return TypeAndParametricEnv{
+      .type = std::move(resolved),
+      .parametric_env = ParametricEnv(parametric_env_map())};
 }
 
 }  // namespace internal

xls/dslx/type_system/parametric_instantiator_internal.h

@@ -103,11 +103,10 @@ class ParametricInstantiator {
     return parametric_default_exprs_;
   }
 
-  absl::Span<const std::string> parametric_order() const {
-    return parametric_order_;
-  }
-
   absl::Span<const InstantiateArg> args() const { return args_; }
+  absl::Span<const ParametricWithType> typed_parametrics() const {
+    return typed_parametrics_;
+  }
 
   const Span& span() const { return span_; }
   DeduceCtx& ctx() { return *ctx_; }
@@ -130,8 +129,7 @@ class ParametricInstantiator {
   // types in this instantiation.
   TypeInfo* derived_type_info_ = nullptr;
 
-  // Notes the iteration order in the original parametric bindings.
-  std::vector<std::string> parametric_order_;
+  absl::Span<const ParametricWithType> typed_parametrics_;
 
   // Note: the expressions may be null (e.g. when the parametric binding has no
   // "default" expression and must be provided by the user).