Merge branch 'topic/bbannier/issue-1938'

CHANGES

@@ -1,3 +1,26 @@
+1.12.0-dev.216 | 2024-12-06 10:34:31 +0100
+
+  * Introduce `deprecated` helper function in Spicy validator. (Benjamin Bannier, Corelight)
+
+  * Add test for using both `[]` and `&count`. (Benjamin Bannier, Corelight)
+
+  * GH-1938: Deprecate `&count` attribute. (Benjamin Bannier, Corelight)
+
+    The preferred way to indicate how many elements should be parsed for a
+    vector has been vector syntax over `&count` for a long time, e.g.,
+
+    ```
+    type X = unit {
+        : uint8[] &count=42; # AVOID.
+        : uint8[42];         # PREFER.
+    };
+    ```
+
+    With this patch `&count` is now deprecated and we emit a warning. We
+    will remove support for `&count` in a future release.
+
+    Closes #1938.
+
 1.12.0-dev.212 | 2024-12-05 10:31:19 +0100
 
   * Fix doc code snippet that won't compile. (Evan Typanski, Corelight)

VERSION

@@ -1 +1 @@
-1.12.0-dev.212
+1.12.0-dev.216

hilti/toolchain/include/compiler/validator.h

@@ -22,6 +22,9 @@ class VisitorMixIn {
     /** Returns the AST context associated with the validator. */
     auto context() const { return _builder->context(); }
 
+    /* Emit a deprecation warning with the given node. */
+    void deprecated(const std::string& msg, const Location& l) const;
+
     /* Record error with given node. */
     void error(std::string msg, Node* n, node::ErrorPriority priority = node::ErrorPriority::Normal);
 

hilti/toolchain/src/compiler/validator.cc

@@ -60,6 +60,10 @@
 using namespace hilti;
 using util::fmt;
 
+void hilti::validator::VisitorMixIn::deprecated(const std::string& msg, const Location& l) const {
+    hilti::logger().deprecated(msg, l);
+}
+
 void validator::VisitorMixIn::error(std::string msg, Node* n, node::ErrorPriority priority) {
     n->addError(std::move(msg), n->location(), priority);
     ++_errors;

spicy/toolchain/src/compiler/validator.cc

@@ -379,7 +379,7 @@ struct VisitorPost : visitor::PreOrder, hilti::validator::VisitorMixIn {
             if ( n->expression() )
                 error("%random-access does not accept an argument", n);
 
-            hilti::logger().deprecated("%random-access is no longer needed and deprecated", n->meta().location());
+            deprecated("%random-access is no longer needed and deprecated", n->meta().location());
         }
 
         else if ( n->id().str() == "%filter" ) {
@@ -799,14 +799,17 @@ struct VisitorPost : visitor::PreOrder, hilti::validator::VisitorMixIn {
         }
 
         if ( count_attr && (repeat && ! repeat->type()->type()->isA<hilti::type::Null>()) )
-            error("cannot have both `[..]` and &count", n);
+            error("cannot have both '[..]' and &count", n);
+
+        if ( count_attr )
+            deprecated("&count=N is deprecated, prefer '[N]' syntax", count_attr->meta().location());
 
         if ( n->attributes()->has(hilti::Attribute::Kind::Convert) &&
              n->attributes()->has(hilti::Attribute::Kind::Chunked) )
-            hilti::logger().deprecated(
+            deprecated(
                 "usage of &convert on &chunked field is ill-defined and deprecated; support will be "
                 "removed in future versions",
-                n);
+                n->meta().location());
 
         if ( n->sinks().size() && ! n->parseType()->type()->isA<hilti::type::Bytes>() )
             error("only a bytes field can have sinks attached", n);
@@ -864,9 +867,8 @@ struct VisitorPost : visitor::PreOrder, hilti::validator::VisitorMixIn {
         if ( auto t = n->itemType()->type()->tryAs<hilti::type::Bitfield>() ) {
             for ( const auto& b : t->bits() ) {
                 if ( b->attributes()->has(hilti::Attribute::Kind::BitOrder) )
-                    hilti::logger().deprecated(fmt("&bit-order on bitfield item '%s' has no effect and is deprecated",
-                                                   b->id()),
-                                               b->meta().location());
+                    deprecated(fmt("&bit-order on bitfield item '%s' has no effect and is deprecated", b->id()),
+                               b->meta().location());
             }
         }
 

tests/Baseline/spicy.types.unit.count-fail-2/output

@@ -1,3 +1,4 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[warning] <...>/count-fail.spicy:5:14-5:26: &count=N is deprecated, prefer '[N]' syntax
 [error] <...>/count-fail.spicy:5:3-5:27: cannot coerce expression 'True' of type 'bool' to type 'uint<64>'
 [error] spicyc: aborting after errors

tests/Baseline/spicy.types.unit.count-fail-3/output

@@ -1,3 +1,4 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[warning] <...>/count-fail.spicy:5:14-5:25: &count=N is deprecated, prefer '[N]' syntax
 [error] <...>/count-fail.spicy:5:3-5:26: cannot coerce expression '0x1p-1' of type 'real' to type 'uint<64>'
 [error] spicyc: aborting after errors

tests/Baseline/spicy.types.unit.count-fail-4/output

@@ -0,0 +1,4 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[warning] <...>/count-fail.spicy:4:16-4:26: &count=N is deprecated, prefer '[N]' syntax
+[error] <...>/count-fail.spicy:4:3-4:27: cannot have both '[..]' and &count
+[error] spicyc: aborting after errors

tests/Baseline/spicy.types.unit.count-fail/output

@@ -1,3 +1,4 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[warning] <...>/count-fail.spicy:10:14-10:24: &count=N is deprecated, prefer '[N]' syntax
 [error] <...>/count-fail.spicy:10:3-10:25: cannot coerce expression '-1' of type 'int<64>' to type 'uint<64>'
 [error] spicyc: aborting after errors

tests/Baseline/spicy.types.unit.count/output

@@ -1,2 +1,4 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[warning] <...>/count.spicy:10:14-10:23: &count=N is deprecated, prefer '[N]' syntax
+[warning] <...>/count.spicy:13:14-13:25: &count=N is deprecated, prefer '[N]' syntax
 [$a=[1], $b=[2]]

tests/spicy/types/unit/count-fail.spicy

@@ -25,3 +25,10 @@ public type U3 = unit() {
   a: uint16;
   b: int64[] &count = 0.5;
 };
+
+# @TEST-START-NEXT
+module testing;
+
+public type U4 = unit () {
+  a: int64[47] &count = 11;
+};

tests/spicy/types/unit/count.spicy

@@ -1,4 +1,4 @@
-# @TEST-EXEC: printf '\00\00\00\00\00\00\00\01\00\00\00\00\00\00\00\02' | spicy-driver %INPUT >output
+# @TEST-EXEC: printf '\00\00\00\00\00\00\00\01\00\00\00\00\00\00\00\02' | spicy-driver %INPUT >output 2>&1
 # @TEST-EXEC: btest-diff output
 
 # `count` accepts unsigned arguments.