Skip to content

Commit

Permalink
Defer start of feature-dependent transformations to a later point.
Browse files Browse the repository at this point in the history
We previously would evaluate whether a unit field or method was required
by an optional feature before we had finished collecting all feature
requirements. This behavior was fine when we were visiting individual
modules' AST on by one, but breaks with #1462 were we changed using a
single AST to hold all modules.

This patch defers transformations until all feature requirements have
been collected.
  • Loading branch information
bbannier committed Jul 16, 2024
1 parent 4a1b43e commit 4892621
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
15 changes: 9 additions & 6 deletions hilti/toolchain/src/compiler/optimizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

#include "hilti/compiler/detail/optimizer.h"

#include <algorithm>
#include <optional>
#include <string>
#include <tuple>
Expand Down Expand Up @@ -200,7 +199,7 @@ struct FunctionVisitor : OptimizerVisitor {

if ( auto type = type_ ) {
for ( const auto& requirement : n->attributes()->findAll("&needed-by-feature") ) {
auto feature = *requirement->valueAsString();
const auto& feature = *requirement->valueAsString();

// If no feature constants were collected yet, reschedule us for the next collection pass.
//
Expand All @@ -212,9 +211,11 @@ struct FunctionVisitor : OptimizerVisitor {
}

auto it = _features.find(type->type()->type()->typeID());
if ( it == _features.end() )
// No feature requirements known for type.
if ( it == _features.end() ) {
// No feature requirements known for type yet.
_collect_again = true;
continue;
}

function.referenced = function.referenced || it->second.at(feature);
}
Expand Down Expand Up @@ -289,9 +290,11 @@ struct FunctionVisitor : OptimizerVisitor {
}

auto it = _features.find(decl->fullyQualifiedID());
if ( it == _features.end() )
// No feature requirements known for type.
if ( it == _features.end() ) {
// No feature requirements known for type yet.
_collect_again = true;
continue;
}

// Mark the function as referenced if it is needed by an active feature.
function.referenced = function.referenced || it->second.at(feature);
Expand Down
20 changes: 20 additions & 0 deletions tests/spicy/optimization/file-order.spicy
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# @TEST-DOC: Checks that the order in which modules are compiled is irrelevant for optimizations, regression test for #1789.
#
# @TEST-EXEC: spicyc -dj y.spicy x.spicy
# @TEST-EXEC: spicyc -dj x.spicy y.spicy

# @TEST-START-FILE x.spicy
module x;

public type X = unit {
on %synced {
confirm;
}
};
# @TEST-END-FILE

# @TEST-START-FILE y.spicy
module y;

type Y = unit {};
# @TEST-END-FILE

0 comments on commit 4892621

Please sign in to comment.