Skip to content

Commit

Permalink
fixup! Functions with locals of structs with lifecycle hooks are not …
Browse files Browse the repository at this point in the history
…pure.
  • Loading branch information
bbannier committed May 16, 2023
1 parent f8378cd commit 7b916a6
Showing 1 changed file with 20 additions and 22 deletions.
42 changes: 20 additions & 22 deletions hilti/toolchain/src/compiler/optimizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1477,6 +1477,18 @@ struct MemberVisitor : OptimizerVisitor, visitor::PreOrder<bool, MemberVisitor>
}
};

// Helper function to detect whether a struct has lifecycle hooks attached.
bool hasLifecycleHooks(const type::Struct& s) {
// NOLINTNEXTLINE(readability-use-anyofallof)
for ( const auto& f : s.fields() ) {
// Currently the only lifecycle hook we have is `~finally`.
if ( f.id().str() == "~finally" )
return true;
}

return false;
};

// Optimizer pass which removes dead store and variable declarations.
struct DeadStoreVisitor : OptimizerVisitor, visitor::PreOrder<bool, DeadStoreVisitor> {
// We use decl identities here instead of their canonical IDs since for
Expand Down Expand Up @@ -1546,10 +1558,9 @@ struct DeadStoreVisitor : OptimizerVisitor, visitor::PreOrder<bool, DeadStoreVis
if ( ! decl.isA<declaration::LocalVariable>() && ! decl.isA<declaration::GlobalVariable>() )
return false;

// Do not work on assignments to values of struct types since structs
// can have hooks attached which could make the assignment result
// visible non-locally.
if ( target->type().isA<type::Struct>() )
// Do not work on assignments to values of struct types which have
// lifecycle hooks attached.
if ( auto s = target->type().tryAs<type::Struct>(); s && hasLifecycleHooks(*s) )
return false;

switch ( _stage ) {
Expand Down Expand Up @@ -1614,9 +1625,9 @@ struct DeadStoreVisitor : OptimizerVisitor, visitor::PreOrder<bool, DeadStoreVis
const auto& decl = x.declaration();

if ( const auto& local = decl.tryAs<declaration::LocalVariable>() ) {
// Do not attempt to remove declarations of structs since
// they might have additional state attached via hooks.
if ( local->type().isA<type::Struct>() )
// Do not attempt to remove declarations of structs with
// lifecycle hooks.
if ( auto s = local->type().tryAs<type::Struct>(); s && hasLifecycleHooks(*s) )
break;

// If the local is initialized do not remove it to still
Expand Down Expand Up @@ -1805,10 +1816,8 @@ struct DeadCodeVisitor : OptimizerVisitor, visitor::PreOrder<bool, DeadCodeVisit
if ( fn->flavor() == type::function::Flavor::Hook )
return true;

// If the function returns a struct type there might be hooks
// like `finally` attached to the type so calling the function
// has side effects.
if ( fn->result().type().isA<type::Struct>() )
// If the function returns a struct type with lifecycle hooks it has side effects.
if ( auto s = fn->result().type().tryAs<type::Struct>(); s && hasLifecycleHooks(*s) )
return true;

// If the function is not marked pure it has side effects.
Expand All @@ -1834,17 +1843,6 @@ struct DeadCodeVisitor : OptimizerVisitor, visitor::PreOrder<bool, DeadCodeVisit
}
};

bool hasLifecycleHooks(const type::Struct& s) {
// NOLINTNEXTLINE(readability-use-anyofallof)
for ( const auto& f : s.fields() ) {
// Currently the only lifecycle hook we have is `~finally`.
if ( f.id().str() == "~finally" )
return true;
}

return false;
};

// An optimizer pass which annotates pure functions. This
// information can be used by other passes to remove dead code.
struct PureFunctionVisitor : OptimizerVisitor, visitor::PreOrder<bool, PureFunctionVisitor> {
Expand Down

0 comments on commit 7b916a6

Please sign in to comment.