Fix non-converging optimizer pass for used functions.

The optimizer's `FunctionVisitor` computes whether a function is used or
required by any feature. Since recently we now hold all modules in a
single AST. This means that we need at least one full pass through the
AST to collect feature constants so we can decided whether a function is
required by a feature and cannot be elided. The initial implementation
of this used explicit control flow, but this is error-prone like seen in

This patch removes the explicit control flow and instead checks whether
we have discovered any new feature constants. With that the optimizer
pass converges again.

Closes #1808.

hilti/toolchain/src/compiler/optimizer.cc

@@ -2,6 +2,7 @@
 
 #include "hilti/compiler/detail/optimizer.h"
 
+#include <numeric>
 #include <optional>
 #include <string>
 #include <tuple>
@@ -100,8 +101,6 @@ struct FunctionVisitor : OptimizerVisitor {
         bool referenced = false;
     };
 
-    bool _collect_again = false;
-
     // Lookup table for feature name -> required.
     using Features = std::map<std::string, bool>;
 
@@ -113,8 +112,17 @@ struct FunctionVisitor : OptimizerVisitor {
     void collect(Node* node) override {
         _stage = Stage::COLLECT;
 
+        // Helper to compute the total number of collected features over all types.
+        auto num_features = [&]() {
+            return std::accumulate(_features.begin(), _features.end(), 0U,
+                                   [](auto acc, auto&& f) { return acc + f.second.size(); });
+        };
+
+        // Whether a function can be elided depends on which features are active. Since we discover features as we visit
+        // the AST (which likely contains multiple modules), we need to iterate until we have collected all features.
         while ( true ) {
-            _collect_again = false;
+            const auto numFeatures0 = num_features();
+
             visitor::visit(*this, node);
 
             if ( logger().isEnabled(logging::debug::OptimizerCollect) ) {
@@ -125,7 +133,10 @@ struct FunctionVisitor : OptimizerVisitor {
                                           uses.hook));
             }
 
-            if ( ! _collect_again )
+            const auto numFeatures1 = num_features();
+
+            // We have seen everything since no new features were found.
+            if ( numFeatures0 == numFeatures1 )
                 break;
         }
     }
@@ -207,14 +218,12 @@ struct FunctionVisitor : OptimizerVisitor {
                         // NOTE: If we emit a `&needed-by-feature` attribute we also always emit a matching feature
                         // constant, so eventually at this point we will see at least one feature constant.
                         if ( _features.empty() ) {
-                            _collect_again = true;
                             return;
                         }
 
                         auto it = _features.find(type->type()->type()->typeID());
                         if ( it == _features.end() || ! it->second.count(feature) ) {
                             // This feature requirement has not yet been collected.
-                            _collect_again = true;
                             continue;
                         }
 
@@ -287,14 +296,12 @@ struct FunctionVisitor : OptimizerVisitor {
                         // NOTE: If we emit a `&needed-by-feature` attribute we also always emit a matching feature
                         // constant, so eventually at this point we will see at least one feature constant.
                         if ( _features.empty() ) {
-                            _collect_again = true;
                             return;
                         }
 
                         auto it = _features.find(decl->fullyQualifiedID());
                         if ( it == _features.end() || ! it->second.count(feature) ) {
                             // This feature requirement has not yet been collected.
-                            _collect_again = true;
                             continue;
                         }
 
@@ -527,8 +534,6 @@ struct TypeVisitor : OptimizerVisitor {
         return isModified();
     }
 
-    // XXX
-
     void operator()(declaration::Field* n) final {
         switch ( _stage ) {
             case Stage::COLLECT: {

tests/spicy/optimization/issue-1808.spicy

@@ -0,0 +1,12 @@
+# @TEST-DOC: Regression test for #1808.
+# We expect this call to terminate.
+# @TEST-EXEC: spicyc -dj %INPUT
+
+module test;
+
+type X = unit {
+    # Field with name identical to type name below.
+    foo: uint8;
+};
+
+type foo = unit {};