Merge remote-tracking branch 'origin/topic/robin/gh-1889-convert-params'

* origin/topic/robin/gh-1889-convert-params: Fix usage of `&convert` with unit's requiring parameters. Factor out logic to validate arguments given to a type.
zeek · Oct 17, 2024 · 156d11f · 156d11f
2 parents d9ccbdf + 9f34500
commit 156d11f
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 31 deletions.
diff --git a/hilti/toolchain/include/compiler/validator.h b/hilti/toolchain/include/compiler/validator.h
@@ -38,6 +38,10 @@ class VisitorMixIn {
     /** Returns the number of errors reported so far. */
     auto errors() const { return _errors; }
 
+    /** Validates if provided type arguments match a type's expectation. */
+    void checkTypeArguments(const node::Range<Expression>& have, const node::Set<type::function::Parameter>& want,
+                            Node* n, bool allow_no_arguments = false, bool do_not_check_types = false);
+
 private:
     Builder* _builder;
     int _errors = 0;

diff --git a/hilti/toolchain/src/compiler/codegen/ctors.cc b/hilti/toolchain/src/compiler/codegen/ctors.cc
@@ -56,7 +56,9 @@ struct Visitor : hilti::visitor::PreOrder {
     void operator()(ctor::Default* n) final {
         std::string args;
 
-        if ( ! n->type()->type()->parameters().empty() ) {
+        // If type arguments are provided, call the corresponding constructor.
+        // If they aren't, we'll use the default constructor instead.
+        if ( ! n->typeArguments().empty() ) {
             auto exprs = cg->compileCallArguments(n->typeArguments(), n->type()->type()->parameters());
             args = util::join(exprs, ", ");
         }

diff --git a/hilti/toolchain/src/compiler/validator.cc b/hilti/toolchain/src/compiler/validator.cc
@@ -81,6 +81,37 @@ void validator::VisitorMixIn::error(std::string msg, Node* n, Location l, node::
     ++_errors;
 }
 
+void validator::VisitorMixIn::checkTypeArguments(const node::Range<Expression>& have,
+                                                 const node::Set<type::function::Parameter>& want, Node* n,
+                                                 bool allow_no_arguments, bool do_not_check_types) {
+    if ( have.size() > want.size() ) {
+        error(fmt("type expects %u parameter%s, but receives %u", want.size(), (want.size() > 1 ? "s" : ""),
+                  have.size()),
+              n);
+    }
+
+    if ( have.empty() && allow_no_arguments )
+        return;
+
+    for ( size_t i = 0; i < want.size(); i++ ) {
+        if ( i < have.size() ) {
+            if ( do_not_check_types )
+                continue;
+
+            if ( type::same(have[i]->type(), want[i]->type()) )
+                continue;
+
+            if ( type::sameExceptForConstness(have[i]->type(), want[i]->type()) && want[i]->type()->isConstant() )
+                continue;
+
+            error(fmt("type expects %s for parameter %u, but receives %s", *want[i]->type(), i + 1, *have[i]->type()),
+                  n);
+        }
+        else if ( ! want[i]->default_() )
+            error(fmt("type parameter %u is missing (%s)", i + 1, want[i]->id()), n);
+    }
+}
+
 namespace {
 struct VisitorPre : visitor::PreOrder, public validator::VisitorMixIn {
     using hilti::validator::VisitorMixIn::VisitorMixIn;
@@ -106,31 +137,6 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
         return Nothing();
     }
 
-    void checkStructArguments(const node::Range<Expression>& have, const node::Set<type::function::Parameter>& want,
-                              Node* n) {
-        if ( have.size() > want.size() ) {
-            error(fmt("type expects %u parameter%s, but receives %u", want.size(), (want.size() > 1 ? "s" : ""),
-                      have.size()),
-                  n);
-        }
-
-        for ( size_t i = 0; i < want.size(); i++ ) {
-            if ( i < have.size() ) {
-                if ( type::same(have[i]->type(), want[i]->type()) )
-                    continue;
-
-                if ( type::sameExceptForConstness(have[i]->type(), want[i]->type()) && want[i]->type()->isConstant() )
-                    continue;
-
-                error(fmt("type expects %s for parameter %u, but receives %s", *want[i]->type(), i + 1,
-                          *have[i]->type()),
-                      n);
-            }
-            else if ( ! want[i]->default_() )
-                error(fmt("type parameter %u is missing (%s)", i + 1, want[i]->id()), n);
-        }
-    }
-
     void operator()(Node* n) final {
         if ( ! n->scope() )
             return;
@@ -232,7 +238,7 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
             }
 
             if ( ! n->type()->type()->parameters().empty() )
-                checkStructArguments(n->typeArguments(), n->type()->type()->parameters(), n);
+                checkTypeArguments(n->typeArguments(), n->type()->type()->parameters(), n);
         }
 
         // Check whether this local variable was declared at module scope. We
@@ -299,7 +305,7 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
         }
 
         if ( ! n->type()->type()->parameters().empty() )
-            checkStructArguments(n->typeArguments(), n->type()->type()->parameters(), n);
+            checkTypeArguments(n->typeArguments(), n->type()->type()->parameters(), n);
     }
 
     ////// Ctors
@@ -316,7 +322,7 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
         }
 
         if ( ! t->parameters().empty() )
-            checkStructArguments(n->typeArguments(), t->parameters(), n);
+            checkTypeArguments(n->typeArguments(), t->parameters(), n, true);
     }
 
     void operator()(hilti::ctor::Exception* n) final {
@@ -737,7 +743,7 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
     // Operators (only special cases here, most validation happens where they are defined)
 
     void operator()(operator_::generic::New* n) final {
-        // We reuse the _checkStructArguments() here, that's why this operator is covered here.
+        // We reuse checkTypeArguments() here, that's why this operator is covered here.
         if ( auto t = n->operands()[0]->type()->type()->tryAs<type::Type_>() ) {
             if ( ! t->typeValue()->type()->parameters().empty() ) {
                 node::Range<Expression> args;
@@ -749,7 +755,7 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
                     args = ctor->as<ctor::Tuple>()->value();
                 }
 
-                checkStructArguments(args, t->typeValue()->type()->parameters(), n);
+                checkTypeArguments(args, t->typeValue()->type()->parameters(), n);
             }
         }
         else if ( ! n->operands()[0]->isA<expression::Ctor>() )

diff --git a/spicy/toolchain/src/compiler/validator.cc b/spicy/toolchain/src/compiler/validator.cc
@@ -879,6 +879,15 @@ struct VisitorPost : visitor::PreOrder, hilti::validator::VisitorMixIn {
 
         if ( auto rc = checkFieldAttributes(n); ! rc )
             error(rc.error(), n);
+
+        if ( auto t = n->type() ) {
+            if ( auto unit = t->type()->tryAs<type::Unit>() )
+                // We disable the actual type checking here because arguments
+                // won't have been coerced yet. We are only interested in in
+                // the number of arguments being correct, type checking will
+                // happen later on the HILTI side.
+                checkTypeArguments(n->arguments(), unit->parameters(), n, false, true);
+        }
     }
 
     void operator()(spicy::type::unit::item::UnresolvedField* n) final {

diff --git a/tests/spicy/types/unit/convert-with-type-param.spicy b/tests/spicy/types/unit/convert-with-type-param.spicy
@@ -0,0 +1,18 @@
+# @TEST-EXEC: spicyc -j -d %INPUT
+#
+# @TEST-DOC: Use `&convert` on a unit that has a type parameter; regression test for #1790.
+
+module Test;
+
+type Ctx = bool;
+
+public type XXX = unit {
+    %context = Ctx;
+    xxx: YYY(self.context());
+};
+
+type YYY = unit(ctx1: Ctx&) {
+    message: ZZZ(ctx1);
+} &convert=self.message;
+
+type ZZZ = unit(ctx2: Ctx&) {};