Merge remote-tracking branch 'origin/topic/etyp/error-anonymous-units'

* origin/topic/etyp/error-anonymous-units:
  Fix possessive "it's" in contexts docs.
  Reject anonymous units in variables and fields.
  Avoid creating external types unnecessarily during AST construction.

CHANGES

@@ -1,3 +1,19 @@
+1.13.0-dev.85 | 2025-02-18 10:04:22 +0100
+
+  * Fix possessive "it's" in contexts docs. (Evan Typanski, Corelight)
+
+  * GH-1966: Reject anonymous units in variables and fields. (Evan Typanski, Corelight)
+
+  * Avoid creating external types unnecessarily during AST construction. (Robin Sommer, Corelight)
+
+    We could end up creating external types for freshly created AST nodes
+    just because we wanted them to have a certain constness/sideness. This
+    could then lead to the original types not being validated because they
+    weren't inserted into the AST anywhere.
+
+    This also extends the AST dump output for qualified types to indicate
+    whether it's an internal or external type.
+
 1.13.0-dev.81 | 2025-02-07 09:21:54 +0100
 
   * Drop freebsd-13 from default CI jobs. (Benjamin Bannier, Corelight)

VERSION

@@ -1 +1 @@
-1.13.0-dev.81
+1.13.0-dev.85

doc/programming/parsing.rst

@@ -2182,7 +2182,7 @@ The batch mode of :ref:`spicy-driver <spicy-driver>` does the same.
     A unit's context value gets set only when a host application uses
     it as the top-level starting point for parsing. If in the above
     example `Foo` wasn't the entry point, but used inside another unit
-    further down during the parsing process, it's context would remain
+    further down during the parsing process, its context would remain
     unset.
 
 As an example, the following grammar---mimicking a request/reply-style

hilti/toolchain/include/ast/type.h

@@ -488,23 +488,23 @@ class QualifiedType : public Node {
 
     /** Factory method creating a copy of the type with "sideness" changed to LHS. */
     auto recreateAsLhs(ASTContext* ctx) const {
-        if ( auto t = _type(); t->isNameType() )
+        if ( auto t = _type(); t->isNameType() && (parent() || t->typeID()) )
             return QualifiedType::createExternal(ctx, t, Constness::Mutable, Side::LHS);
         else
             return QualifiedType::create(ctx, t, Constness::Mutable, Side::LHS);
     }
 
     /** Factory method creating a copy of the type with constness changed to constant. */
     auto recreateAsConst(ASTContext* ctx) const {
-        if ( auto t = _type(); t->isNameType() )
+        if ( auto t = _type(); t->isNameType() && (parent() || t->typeID()) )
             return QualifiedType::createExternal(ctx, t, Constness::Const, Side::RHS);
         else
             return QualifiedType::create(ctx, t, Constness::Const, Side::RHS);
     }
 
     /** Factory method creating a copy of the type with constness changed to non-constant. */
     auto recreateAsNonConst(ASTContext* ctx) const {
-        if ( auto t = _type(); t->isNameType() )
+        if ( auto t = _type(); t->isNameType() && (parent() || t->typeID()) )
             return QualifiedType::createExternal(ctx, t, Constness::Mutable, Side::RHS);
         else
             return QualifiedType::create(ctx, t, Constness::Mutable, Side::RHS);

hilti/toolchain/src/ast/type.cc

@@ -91,7 +91,8 @@ type::Name* QualifiedType::alias() const { return _type()->tryAs<type::Name>();
 hilti::node::Properties QualifiedType::properties() const {
     auto side = (_side == Side::LHS ? "lhs" : "rhs");
     auto constness = (_constness == Constness::Const ? "true" : "false");
-    return {{"const", constness}, {"side", side}};
+    auto external = (_external ? "true" : "false");
+    return {{"const", constness}, {"side", side}, {"extern", external}};
 }
 
 std::string QualifiedType::_dump() const {

hilti/toolchain/src/compiler/validator.cc

@@ -177,6 +177,16 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
         return Nothing();
     }
 
+    // Ensures the declaration's type is a valid type.
+    void checkDeclarationType(Declaration* decl, QualifiedType* ty) {
+        if ( ty->type()->isA<hilti::type::Struct>() || ty->type()->isA<hilti::type::Enum>() ||
+             ty->type()->isA<hilti::type::Union>() ) {
+            if ( ! ty->type()->typeID() )
+                error(fmt("%s types must be named in declarations", ty->type()->typeClass()), decl,
+                      node::ErrorPriority::High);
+        }
+    }
+
     void operator()(Node* n) final {
         if ( ! n->scope() )
             return;
@@ -236,6 +246,8 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
     }
 
     void operator()(declaration::Constant* n) final {
+        checkDeclarationType(n, n->type());
+
         if ( n->value()->type()->isWildcard() )
             error("cannot use wildcard type for constants", n);
 
@@ -249,6 +261,8 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
         hilti::visitor::visit(VisitExpressions(this), n);
     }
 
+    void operator()(declaration::Field* n) final { checkDeclarationType(n, n->type()); }
+
     void operator()(declaration::Function* n) final {
         if ( ! operator_::registry().byBuiltinFunctionID(n->id().local()).empty() )
             error("function uses reserved ID", n);
@@ -258,6 +272,8 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
     }
 
     void operator()(declaration::LocalVariable* n) final {
+        checkDeclarationType(n, n->type());
+
         if ( auto t = n->type()->type();
              ! t->isAllocable() && ! t->isA<type::Unknown>() ) // unknown will be reported elsewhere
             error(fmt("type '%s' cannot be used for variable declaration", *n->type()), n);
@@ -299,6 +315,7 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
 
     void operator()(declaration::Parameter* n) final {
         checkNodeAttributes(n->nodeTag(), n->attributes(), n->displayName());
+        checkDeclarationType(n, n->type());
 
         if ( ! n->type()->type()->isA<type::Auto>() ) {
             if ( ! n->type()->type()->isAllocable() && ! n->type()->type()->isA<type::Any>() )
@@ -333,6 +350,8 @@ struct VisitorPost : visitor::PreOrder, public validator::VisitorMixIn {
     }
 
     void operator()(declaration::GlobalVariable* n) final {
+        checkDeclarationType(n, n->type());
+
         if ( auto t = n->type()->type();
              ! t->isAllocable() && ! t->isA<type::Unknown>() ) // unknown will be reported elsewhere
             error(fmt("type '%s' cannot be used for variable declaration", *n->type()), n);

spicy/toolchain/src/compiler/validator.cc

@@ -769,6 +769,11 @@ struct VisitorPost : visitor::PreOrder, hilti::validator::VisitorMixIn {
     void operator()(spicy::type::Unit* n) final {
         checkNodeAttributes(n->nodeTag(), n->attributes(), "unit type");
 
+        if ( ! n->typeID() ) {
+            error("unit types must be named", n);
+            return;
+        }
+
         if ( auto attrs = n->attributes() ) {
             if ( attrs->has(hilti::attribute::Kind::Size) && attrs->has(hilti::attribute::Kind::MaxSize) )
                 error(("attributes cannot be combined: &size, &max-size"), n);
@@ -825,13 +830,11 @@ struct VisitorPost : visitor::PreOrder, hilti::validator::VisitorMixIn {
         if ( auto contexts = n->propertyItems("%context"); contexts.size() > 1 )
             error("unit cannot have more than one %context", n);
 
-        if ( const auto& type_id = n->typeID() ) {
-            const auto& type_name = type_id.local();
-            for ( const auto& item : n->items() )
-                if ( auto field = item->tryAs<spicy::type::unit::item::Field>(); field && field->id() == type_name )
-                    error(fmt("field name '%s' cannot have name identical to owning unit '%s'", field->id(), type_id),
-                          n);
-        }
+        const auto& type_id = n->typeID();
+        const auto& type_name = type_id.local();
+        for ( const auto& item : n->items() )
+            if ( auto field = item->tryAs<spicy::type::unit::item::Field>(); field && field->id() == type_name )
+                error(fmt("field name '%s' cannot have name identical to owning unit '%s'", field->id(), type_id), n);
 
         if ( n->propertyItem("%synchronize-at") && n->propertyItem("%synchronize-after") )
             error("unit cannot specify both %synchronize-at and %synchronize-after", n);