Add peephole optimizer for final AST tuning.

We use this to remove two statement constructs that the main optimizer may leave
behind:

    1. `default<void>()`
    2. `(*self).__error = __error; __error = (*self).__error;`

The second case is a quite specific situation that eventually, once we
have CFG/DFG tracking, the main optimizer should be able to cover more
generically. However, for now, it's just not nice to always have
these blocks in the generated C++ code, so adding this special case
seems useful.

hilti/toolchain/include/ast/node.h

@@ -456,6 +456,26 @@ class Node {
         return n;
     }
 
+    /**
+     * Returns the subsequent sibling of given child node. This skips over null children.
+     *
+     * @param n child whose sibling to return
+     * @return sibling of *n*, or null if *n* is the last child or not child at all
+     **/
+    Node* sibling(Node* n) const {
+        auto i = std::find(_children.begin(), _children.end(), n);
+        if ( i == _children.end() )
+            return nullptr;
+
+        while ( true ) {
+            if ( ++i == _children.end() )
+                return nullptr;
+
+            if ( *i )
+                return *i;
+        }
+    }
+
     /**
      * Adds a child node. The node will be appended to the end of the current
      * list of children, and its parent will be set to the current node. If the

hilti/toolchain/src/compiler/optimizer.cc

@@ -907,6 +907,126 @@ struct ConstantFoldingVisitor : OptimizerVisitor {
     }
 };
 
+/**
+ * Visitor running on the final, optimized AST to perform additional peephole
+ * optimizations. Will run repeatedly until no further optimizations.
+ */
+struct PeepholeOptimizer : visitor::MutatingPostOrder {
+    using visitor::MutatingPostOrder::MutatingPostOrder;
+
+    // Returns true if statement is `(*self).__error = __error`.
+    bool isErrorPush(statement::Expression* n) {
+        auto assign = n->expression()->tryAs<expression::Assign>();
+        if ( ! assign )
+            return false;
+
+        auto lhs = assign->target()->tryAs<operator_::struct_::MemberNonConst>();
+        if ( ! lhs )
+            return false;
+
+        auto op0 = lhs->op0()->tryAs<operator_::value_reference::Deref>();
+        if ( ! op0 )
+            return false;
+
+        auto op1 = lhs->op1()->tryAs<expression::Member>();
+        if ( ! (op1 && op1->id() == "__error") )
+            return false;
+
+        auto self = op0->op0()->tryAs<expression::Name>();
+        if ( ! (self && self->id() == "self") )
+            return false;
+
+        auto rhs = assign->source()->tryAs<expression::Name>();
+        if ( ! (rhs && rhs->id() == "__error") )
+            return false;
+
+        return true;
+    }
+
+    // Returns true if statement is `__error == (*self).__error`.
+    bool isErrorPop(statement::Expression* n) {
+        auto assign = n->expression()->tryAs<expression::Assign>();
+        if ( ! assign )
+            return false;
+
+        auto lhs = assign->target()->tryAs<expression::Name>();
+        if ( ! (lhs && lhs->id() == "__error") )
+            return false;
+
+        auto rhs = assign->source()->tryAs<operator_::struct_::MemberNonConst>();
+        if ( ! rhs )
+            return false;
+
+        auto op0 = rhs->op0()->tryAs<operator_::value_reference::Deref>();
+        if ( ! op0 )
+            return false;
+
+        auto op1 = rhs->op1()->tryAs<expression::Member>();
+        if ( ! (op1 && op1->id() == "__error") )
+            return false;
+
+        auto self = op0->op0()->tryAs<expression::Name>();
+        if ( ! (self && self->id() == "self") )
+            return false;
+
+        return true;
+    }
+
+    void operator()(statement::Expression* n) final {
+        // Remove expression statements that are just `default<void>`.
+        if ( auto ctor = n->expression()->tryAs<expression::Ctor>();
+             ctor && ctor->ctor()->isA<ctor::Default>() && ctor->type()->type()->isA<type::Void>() ) {
+            recordChange(n, "removing default<void> statement");
+            n->parent()->removeChild(n);
+            return;
+        }
+
+        // Remove statement pairs of the form:
+        //
+        //    (*self).__error = __error;
+        //    __error = (*self).__error;
+        //
+        // These will be left behind by the optimizer if a hook call got optimized out in between them.
+        if ( isErrorPush(n) && n->parent() ) {
+            auto parent = n->parent();
+            if ( auto sibling = parent->sibling(n) ) {
+                if ( auto stmt = sibling->tryAs<statement::Expression>(); stmt && isErrorPop(stmt) ) {
+                    recordChange(n, "removing unneeded error push/pop statements");
+                    parent->removeChild(n);
+                    parent->removeChild(sibling);
+                    return;
+                }
+            }
+        }
+    }
+};
+
+/*
+(*self).__error = __error;
+__error = (*self).__error;
+- statement::Expression [parent @s:600002ac3d40] [@s:600002af86e0]
+[debug/ast-final]                   - expression::Assign [parent @s:600002af86e0]  (non-const)  (resolved)
+[@e:600002af8690] [debug/ast-final]                     - operator_::struct_::MemberNonConst <kind="."> [parent
+@e:600002af8690]  (non-const)  (resolved) [@o:600002d9b000] [debug/ast-final]                       - QualifiedType
+<const="false" side="lhs"> [parent @o:600002d9b000] [@q:60000227a840] [debug/ast-final]                         -
+type::Optional <declaration="-" type="-" unified="optional(name(hilti::RecoverableFailure))" wildcard="false"> [parent
+@q:60000227a840] (resolved) [@t:600003bb> [debug/ast-final]                           - QualifiedType <const="true"
+side="rhs"> [parent @t:600003bb2490] [@q:600002279880] [debug/ast-final]                             - type::Name
+<builtin="false" declaration="-" id="hilti::RecoverableFailure" resolved-type="T32" type="-"
+unified="name(hilti::RecoverableFailure)" wildcard=> [debug/ast-final]                       -
+operator_::value_reference::Deref <auto="true" kind="*"> [parent @o:600002d9b000]  (non-const)  (resolved)
+[@o:600002d9afa0] [debug/ast-final]                         - QualifiedType <const="false" side="rhs"> [parent
+@o:600002d9afa0] [@q:60000227aed0] [debug/ast-final]                         - expression::Name <fqid="" id="self"
+resolved-declaration="D114" resolved-unified="value_ref(name(Test::X))"> [parent @o:600002d9afa0]  (non-const)
+(resolved) > [debug/ast-final]                       - expression::Member <id="__error"> [parent @o:600002d9b000]
+(const)  (resolved) [@e:600002279ce0] [debug/ast-final]                         - QualifiedType <const="true"
+side="rhs"> [parent @e:600002279ce0] [@q:600002279e30] [debug/ast-final]                           - type::Member
+<declaration="-" id="__error" type="-" unified="member(__error)" wildcard="false"> [parent @q:600002279e30] (resolved)
+[@t:600003ec6d60] [debug/ast-final]                     - expression::Name <fqid="" id="__error"
+resolved-declaration="D67" resolved-unified="optional(name(hilti::RecoverableFailure))"> [parent @e:600002af8690]
+(non-cons
+*/
+
 // This visitor collects requirement attributes in the AST and toggles unused features.
 class FeatureRequirementsVisitor : public visitor::MutatingPreOrder {
 public:
@@ -1494,6 +1614,13 @@ void detail::optimizer::optimize(Builder* builder, ASTRoot* root) {
         ++round;
     }
 
+    while ( true ) {
+        auto v = PeepholeOptimizer(builder, hilti::logging::debug::Optimizer);
+        visitor::visit(v, root);
+        if ( ! v.isModified() )
+            break;
+    }
+
     // Clear cached information which might become outdated due to edits.
     auto v = hilti::visitor::PreOrder();
     for ( auto n : hilti::visitor::range(v, root, {}) )

tests/Baseline/hilti.optimization.unimplemented_hook/log

@@ -11,3 +11,5 @@
 [debug/optimizer] [unimplemented_hook.hlt:21:5-21:35] declaration::Field "hook void unimplemented_void();" -> null
 [debug/optimizer] removing field for unused method Foo::X::unimplemented_int64
 [debug/optimizer] [unimplemented_hook.hlt:22:5-22:49] declaration::Field "hook optional<int<64>> unimplemented_int64();" -> null
+[debug/optimizer] [unimplemented_hook.hlt:29:1-29:27] statement::Expression "default<void>();" -> removing default<void> statement
+[debug/optimizer] [unimplemented_hook.hlt:35:1-35:22] statement::Expression "default<void>();" -> removing default<void> statement

tests/Baseline/hilti.optimization.unimplemented_hook/opt.hlt

@@ -18,8 +18,6 @@ method hook void X::implemented() {
 }
 
 global_implemented();
-default<void>();
 x.implemented();
-default<void>();
 
 }

tests/Baseline/spicy.optimization.default-parser-functions/log

@@ -174,6 +174,15 @@
 [debug/optimizer] [<no location>] expression::Name "__feat%foo@@P2%uses_stream" -> expression::Ctor "False" (inlining constant)
 [debug/optimizer] [<no location>] expression::Ternary "False ? (*self).__filters : Null" -> expression::Ctor "Null"
 [debug/optimizer] [<no location>] expression::Ternary "False ? (*self).__filters : Null" -> expression::Ctor "Null"
+[debug/optimizer] [<no location>] statement::Expression "(*self).__error = __error;" -> removing unneeded error push/pop statements
+[debug/optimizer] [<no location>] statement::Expression "(*self).__error = __error;" -> removing unneeded error push/pop statements
+[debug/optimizer] [<no location>] statement::Expression "(*self).__error = __error;" -> removing unneeded error push/pop statements
+[debug/optimizer] [<no location>] statement::Expression "(*self).__error = __error;" -> removing unneeded error push/pop statements
+[debug/optimizer] [<no location>] statement::Expression "(*self).__error = __error;" -> removing unneeded error push/pop statements
+[debug/optimizer] [<no location>] statement::Expression "(*self).__error = __error;" -> removing unneeded error push/pop statements
+[debug/optimizer] [<no location>] statement::Expression "(*self).__error = __error;" -> removing unneeded error push/pop statements
+[debug/optimizer] [<no location>] statement::Expression "(*self).__error = __error;" -> removing unneeded error push/pop statements
+[debug/optimizer] [<no location>] statement::Expression "(*self).__error = __error;" -> removing unneeded error push/pop statements
 [debug/optimizer] [<no location>] statement::If "if ( False ) { (*__unit).__begin = begin(__ncur); }" -> null
 [debug/optimizer] [<no location>] statement::If "if ( False ) { (*__unit).__begin = begin(__ncur); }" -> null
 [debug/optimizer] [<no location>] statement::If "if ( False ) { (*__unit).__begin = begin(__ncur); }" -> null
@@ -330,6 +339,11 @@
 [debug/optimizer] [default-parser-functions.spicy:14:18-14:24] operator_::struct_::MemberCall "(*self).__on_0x25_finally()" -> expression::Ctor "default<void>()" (replacing call to unimplemented method with default value)
 [debug/optimizer] [default-parser-functions.spicy:14:18-14:24] operator_::struct_::MemberCall "(*self).__on_0x25_finally()" -> expression::Ctor "default<void>()" (replacing call to unimplemented method with default value)
 [debug/optimizer] [default-parser-functions.spicy:14:18-14:24] operator_::struct_::MemberCall "(*self).__on_0x25_init()" -> expression::Ctor "default<void>()" (replacing call to unimplemented method with default value)
+[debug/optimizer] [default-parser-functions.spicy:14:18-14:24] statement::Expression "default<void>();" -> removing default<void> statement
+[debug/optimizer] [default-parser-functions.spicy:14:18-14:24] statement::Expression "default<void>();" -> removing default<void> statement
+[debug/optimizer] [default-parser-functions.spicy:14:18-14:24] statement::Expression "default<void>();" -> removing default<void> statement
+[debug/optimizer] [default-parser-functions.spicy:14:18-14:24] statement::Expression "default<void>();" -> removing default<void> statement
+[debug/optimizer] [default-parser-functions.spicy:14:18-14:24] statement::Expression "default<void>();" -> removing default<void> statement
 [debug/optimizer] [default-parser-functions.spicy:16:18-21:1] declaration::Field "hook optional<string> __str__();" -> null
 [debug/optimizer] [default-parser-functions.spicy:16:18-21:1] declaration::Field "hook void __on_0x25_confirmed() &needed-by-feature="synchronization";" -> null
 [debug/optimizer] [default-parser-functions.spicy:16:18-21:1] declaration::Field "hook void __on_0x25_done();" -> null
@@ -349,8 +363,13 @@
 [debug/optimizer] [default-parser-functions.spicy:16:18-21:1] operator_::struct_::MemberCall "(*self).__on_0x25_finally()" -> expression::Ctor "default<void>()" (replacing call to unimplemented method with default value)
 [debug/optimizer] [default-parser-functions.spicy:16:18-21:1] operator_::struct_::MemberCall "(*self).__on_0x25_finally()" -> expression::Ctor "default<void>()" (replacing call to unimplemented method with default value)
 [debug/optimizer] [default-parser-functions.spicy:16:18-21:1] operator_::struct_::MemberCall "(*self).__on_0x25_init()" -> expression::Ctor "default<void>()" (replacing call to unimplemented method with default value)
+[debug/optimizer] [default-parser-functions.spicy:16:18-21:1] statement::Expression "default<void>();" -> removing default<void> statement
+[debug/optimizer] [default-parser-functions.spicy:16:18-21:1] statement::Expression "default<void>();" -> removing default<void> statement
+[debug/optimizer] [default-parser-functions.spicy:16:18-21:1] statement::Expression "default<void>();" -> removing default<void> statement
+[debug/optimizer] [default-parser-functions.spicy:16:18-21:1] statement::Expression "default<void>();" -> removing default<void> statement
 [debug/optimizer] [default-parser-functions.spicy:17:5-17:13] declaration::Field "hook void __on_x(uint<8> __dd);" -> null
 [debug/optimizer] [default-parser-functions.spicy:17:5-17:13] operator_::struct_::MemberCall "(*self).__on_x((*self).x)" -> expression::Ctor "default<void>()" (replacing call to unimplemented method with default value)
+[debug/optimizer] [default-parser-functions.spicy:17:5-17:13] statement::Expression "default<void>();" -> removing default<void> statement
 [debug/optimizer] [spicy_rt.hlt:28:5-28:76] declaration::Field "method void connect_mime_type(string mime_type, string scope) &internal;" -> null (removing unused member)
 [debug/optimizer] [spicy_rt.hlt:29:5-29:75] declaration::Field "method void connect_mime_type(bytes mime_type, string scope) &internal;" -> null (removing unused member)
 [debug/optimizer] removing field for unused method foo::P0::__on_0x25_confirmed

tests/Baseline/spicy.optimization.default-parser-functions/opt.hlt

@@ -61,35 +61,21 @@ method method tuple<const view<stream>, int<64>, const iterator<stream>, optiona
     try {
         hilti::debugIndent("spicy");
         local iterator<stream> __begin = begin(__cur);
-        (*self).__error = __error;
-        default<void>();
-        __error = (*self).__error;
         local strong_ref<stream> filtered = Null;
 
         if ( ! filtered ) 
             __result = (*self).__parse_foo__P1_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error);
 
     }
-    catch ( hilti::SystemException __except ) {
-        default<void>();
-        (*self).__error = __error;
-        default<void>();
-        __error = (*self).__error;
+    catch ( hilti::SystemException __except ) 
         throw;
-    }
 
-    (*self).__error = __error;
-    default<void>();
-    __error = (*self).__error;
     return __result;
 }
 
 method method tuple<const view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> foo::P1::__parse_foo__P1_stage2(inout value_ref<stream> __data, iterator<stream> __begin, copy view<stream> __cur, copy bool __trim, copy int<64> __lah, copy iterator<stream> __lahe, copy optional<hilti::RecoverableFailure> __error) {
     # "<...>/default-parser-functions.spicy:14:18-14:24"
     local tuple<view<stream>, int<64>, const iterator<stream>, optional<hilti::RecoverableFailure>> __result;
-    (*self).__error = __error;
-    default<void>();
-    __error = (*self).__error;
     hilti::debugDedent("spicy");
     __result = (__cur, __lah, __lahe, __error);
     return __result;
@@ -169,9 +155,6 @@ method method tuple<const view<stream>, int<64>, const iterator<stream>, optiona
     try {
         hilti::debugIndent("spicy");
         local iterator<stream> __begin = begin(__cur);
-        (*self).__error = __error;
-        default<void>();
-        __error = (*self).__error;
         local strong_ref<stream> filtered = Null;
 
         if ( ! filtered ) 
@@ -180,15 +163,9 @@ method method tuple<const view<stream>, int<64>, const iterator<stream>, optiona
     }
     catch ( hilti::SystemException __except ) {
         (*self).__on_0x25_error(hilti::exception_what(__except));
-        (*self).__error = __error;
-        default<void>();
-        __error = (*self).__error;
         throw;
     }
 
-    (*self).__error = __error;
-    default<void>();
-    __error = (*self).__error;
     return __result;
 }
 
@@ -206,9 +183,6 @@ method method tuple<const view<stream>, int<64>, const iterator<stream>, optiona
 
     # End parsing production: Variable: x   -> uint<8>
 
-    (*self).__error = __error;
-    default<void>();
-    __error = (*self).__error;
     # "<...>/default-parser-functions.spicy:18:8-18:12"
 
     # Begin parsing production: Variable: y   -> uint<8>
@@ -223,9 +197,6 @@ method method tuple<const view<stream>, int<64>, const iterator<stream>, optiona
     (*self).__error = __error;
     (*self).__on_y((*self).y);
     __error = (*self).__error;
-    (*self).__error = __error;
-    default<void>();
-    __error = (*self).__error;
     hilti::debugDedent("spicy");
     __result = (__cur, __lah, __lahe, __error);
     return __result;