Add support for field-local %error handlers.

We now support attaching an `%error` handler to an individual field: type Test = unit { a: b"A"; b: b"B" %error { print "field B %error", self; } c: b"C"; }; With input `AxC`, that handler will trigger, whereas with `ABx` it won't. If the unit had a unit-wide `%error` handler as well, that one would trigger in both cases (i.e., for `b`, in addition to its field local handler). The handler can also be provided separately from the field: on b %error { ... } In that separate version, one can receive the error message as well by declaring a corresponding string parameter: on b(msg: string) %error { ... } This works externally, from outside the unit, as well: on Test::b(msg: string) %error { ... } This is eebased on top of `topic/robin/optimize-type-parsing` so that we get the peephole optimizer. Addresses #1824.
zeek · Sep 24, 2024 · 83c5dcc · 83c5dcc
1 parent 5a280eb
commit 83c5dcc
Show file tree

Hide file tree

Showing 23 changed files with 397 additions and 154 deletions.
diff --git a/spicy/toolchain/include/ast/declarations/hook.h b/spicy/toolchain/include/ast/declarations/hook.h
@@ -2,10 +2,7 @@
 
 #pragma once
 
-#include <memory>
-#include <string>
 #include <utility>
-#include <vector>
 
 #include <hilti/ast/expressions/keyword.h>
 #include <hilti/ast/function.h>
@@ -42,12 +39,16 @@ enum class Type {
 
     /** `foreach` hook for containers, executing for each element added. */
     ForEach,
+
+    /** `%error` hook executing when an error has occurred processing the field. */
+    Error,
 };
 
 namespace detail {
 constexpr hilti::util::enum_::Value<Type> Types[] = {
     {Type::Standard, "standard"},
     {Type::ForEach, "foreach"},
+    {Type::Error, "error"},
 };
 
 } // namespace detail
@@ -81,6 +82,8 @@ class Hook : public Declaration {
     hook::Type hookType() const {
         if ( attributes()->has("foreach") )
             return hook::Type::ForEach;
+        else if ( attributes()->has("%error") )
+            return hook::Type::Error;
         else
             return hook::Type::Standard;
     }

diff --git a/spicy/toolchain/include/ast/declarations/unit-hook.h b/spicy/toolchain/include/ast/declarations/unit-hook.h
@@ -2,8 +2,6 @@
 
 #pragma once
 
-#include <memory>
-#include <string>
 #include <utility>
 
 #include <hilti/ast/declaration.h>

diff --git a/spicy/toolchain/src/compiler/codegen/codegen.cc b/spicy/toolchain/src/compiler/codegen/codegen.cc
@@ -631,6 +631,10 @@ hilti::declaration::Function* CodeGen::compileHook(const type::Unit& unit, const
             builder()->parameter("__dd", field->ddType()->type()->elementType()->type(), hilti::parameter::Kind::In));
         params.push_back(builder()->parameter("__stop", builder()->typeBool(), hilti::parameter::Kind::InOut));
     }
+    else if ( type == declaration::hook::Type::Error ) {
+        if ( params.empty() )
+            params.push_back(builder()->parameter("__excpt", builder()->typeString(), hilti::parameter::Kind::In));
+    }
     else if ( original_field_type ) {
         params.push_back(builder()->parameter("__dd", field->itemType()->type(), hilti::parameter::Kind::In));
 
@@ -652,10 +656,20 @@ hilti::declaration::Function* CodeGen::compileHook(const type::Unit& unit, const
         hid = "__str__";
     }
     else {
+        std::string postfix;
+
+        switch ( type ) {
+            case declaration::hook::Type::Standard: break;
+            case declaration::hook::Type::Error: postfix = "_error"; break;
+            case declaration::hook::Type::ForEach: postfix = "_foreach"; break;
+        }
+
+        hid = fmt("__on_%s%s", id.local(), postfix);
         result = builder()->qualifiedType(builder()->typeVoid(), hilti::Constness::Const);
-        hid = fmt("__on_%s%s", id.local(), (type == declaration::hook::Type::ForEach ? "_foreach" : ""));
     }
 
+    assert(! hid.empty());
+
     if ( ! id.namespace_().empty() )
         hid = fmt("%s::%s", id.namespace_(), hid);
 

diff --git a/spicy/toolchain/src/compiler/codegen/parser-builder.cc b/spicy/toolchain/src/compiler/codegen/parser-builder.cc
@@ -578,6 +578,24 @@ struct ProductionVisitor : public production::Visitor {
 
         // Parse production
 
+        std::optional<std::pair<std::shared_ptr<Builder>, Builder::TryProxy>> try_;
+
+        if ( is_field_owner ) {
+            bool has_error_hook = false;
+            for ( const auto* h : field->hooks() ) {
+                if ( h->hookType() == declaration::hook::Type::Error )
+                    has_error_hook = true;
+            }
+
+            if ( has_error_hook || ! field->isAnonymous() ) {
+                // Wrap field parsing into try-block for per-field %error hook,
+                // which may be attached directly or provided externally for
+                // named fields.
+                try_ = builder()->addTry();
+                pushBuilder(try_->first);
+            }
+        }
+
         builder()->setLocation(p->location());
 
         Expression* pre_container_offset = nullptr;
@@ -664,6 +682,20 @@ struct ProductionVisitor : public production::Visitor {
             path_tracker.reset();
         }
 
+        if ( try_ ) {
+            popBuilder(); // per-field try-block
+
+            auto catch_ =
+                try_->second.addCatch(builder()->parameter("__except", builder()->typeName("hilti::SystemException")));
+
+            pushBuilder(catch_, [&]() {
+                auto what = builder()->call("hilti::exception_what", {builder()->id("__except")});
+                builder()->addMemberCall(state().self, ID(fmt("__on_%s_error", field->id().local())), {what},
+                                         field->meta());
+                builder()->addRethrow();
+            });
+        }
+
         // Top of stack will now have the final value for the field.
         Expression* stop = builder()->bool_(false);
 

diff --git a/spicy/toolchain/src/compiler/codegen/unit-builder.cc b/spicy/toolchain/src/compiler/codegen/unit-builder.cc
@@ -77,6 +77,7 @@ struct FieldBuilder : public visitor::PreOrder {
 
         if ( f->emitHook() ) {
             add_hook_declaration(f, declaration::hook::Type::Standard);
+            add_hook_declaration(f, declaration::hook::Type::Error);
 
             if ( f->isContainer() )
                 add_hook_declaration(f, declaration::hook::Type::ForEach);

diff --git a/spicy/toolchain/src/compiler/parser/parser.yy b/spicy/toolchain/src/compiler/parser/parser.yy
@@ -35,7 +35,7 @@ namespace spicy { namespace detail { class Parser; } }
 %verbose
 
 %glr-parser
-%expect 94
+%expect 100
 %expect-rr 168
 
 %{
@@ -902,6 +902,7 @@ unit_hook_attribute
               : FOREACH                          { $$ = builder->attribute("foreach", __loc__); }
               | PRIORITY '=' expr                { $$ = builder->attribute("&priority", std::move($3), __loc__); }
               | PROPERTY                         { $$ = builder->attribute($1, __loc__); }
+              | attribute                        { $$ = std::move($1); }
 
 unit_switch   : SWITCH opt_unit_switch_expr '{' unit_switch_cases '}' opt_attributes opt_unit_field_condition ';'
                                                  { $$ = builder->typeUnitItemSwitch(std::move($2), std::move($4), std::move($7), {}, std::move($6), __loc__); }

diff --git a/spicy/toolchain/src/compiler/resolver.cc b/spicy/toolchain/src/compiler/resolver.cc
@@ -131,9 +131,7 @@ struct VisitorPass2 : visitor::MutatingPostOrder {
     }
 
     void operator()(declaration::UnitHook* n) final {
-        if ( ! n->fullyQualifiedID() ) {
-            if ( auto m = n->parent<hilti::declaration::Module>() )
-                n->setFullyQualifiedID(m->id() + n->id()); // global scope
+        if ( n->hook()->hookType() == declaration::hook::Type::Error ) {
         }
     }
 

diff --git a/spicy/toolchain/src/compiler/validator.cc b/spicy/toolchain/src/compiler/validator.cc
@@ -479,16 +479,22 @@ struct VisitorPost : visitor::PreOrder, hilti::validator::VisitorMixIn {
     }
 
     void operator()(spicy::declaration::Hook* n) final {
-        for ( const auto* attr : n->attributes()->attributes() ) {
-            if ( attr->tag() == "foreach" ) {
+        int count = 0;
+        switch ( n->hookType() ) {
+            case declaration::hook::Type::ForEach:
                 if ( auto field = n->parent<spicy::type::unit::item::Field>(); field && ! field->isContainer() )
                     error("'foreach' can only be used with containers", n);
-            }
-            else if ( attr->tag() == "%debug" || attr->tag() == "&priority" )
-                continue;
-            else
-                error(fmt("unknown hook type '%s'", attr->tag()), n);
+
+                ++count;
+                break;
+
+            case declaration::hook::Type::Error: ++count; break;
+
+            default: break;
         }
+
+        if ( count > 1 )
+            error("hook cannot have be both types, 'foreach' and '%error'", n);
     }
 
     void operator()(spicy::type::unit::item::UnitHook* n) final {
@@ -1018,6 +1024,12 @@ struct VisitorPost : visitor::PreOrder, hilti::validator::VisitorMixIn {
                           id_readable),
                       n, location);
         }
+
+        else if ( hook->hookType() == declaration::hook::Type::Error && ! params.empty() ) {
+            if ( params.size() != 1 || ! hilti::type::same(params[0]->type()->type(), builder()->typeString()) )
+                error("%error hook must only take a string parameter", n, location);
+        }
+
         else {
             if ( auto i = unit->itemByName(ID(id)); ! i )
                 error(fmt("no field '%s' in unit type", id), n, location);

diff --git a/tests/Baseline/spicy.optimization.default-parser-functions/log b/tests/Baseline/spicy.optimization.default-parser-functions/log
@@ -273,6 +273,10 @@
 [debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
 [debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
 [debug/optimizer] [<no location>] statement::If "if ( False ) { }" -> null
+[debug/optimizer] [<no location>] statement::Try "try { # "<...>/default-parser-functions.spicy:17:8-17:12" # Begin parsing production: Variable: x -> uint<8> spicy_rt::waitForInput(__data, __cur, 1, "expecting 1 bytes for unpacking value", "<...>/default-parser-functions.spicy:17:8-17:12", Null); ((*self).x, __cur) = (*unpack<uint<8>>((__cur, hilti::ByteOrder::Network))); if ( __trim ) (*__data).trim(begin(__cur)); # End parsing production: Variable: x -> uint<8> (*self).__error = __error; __error = (*self).__error; } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
+[debug/optimizer] [<no location>] statement::Try "try { # "<...>/default-parser-functions.spicy:17:8-17:12" # Begin parsing production: Variable: x -> uint<8> spicy_rt::waitForInput(__data, __cur, 1, "expecting 1 bytes for unpacking value", "<...>/default-parser-functions.spicy:17:8-17:12", Null); ((*self).x, __cur) = (*unpack<uint<8>>((__cur, hilti::ByteOrder::Network))); if ( __trim ) (*__data).trim(begin(__cur)); # End parsing production: Variable: x -> uint<8> (*self).__error = __error; __error = (*self).__error; } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ # "<...>/default-parser-functions.spicy:17:8-17:12" # Begin parsing production: Variable: x -> uint<8> spicy_rt::waitForInput(__data, __cur, 1, "expecting 1 bytes for unpacking value", "<...>/default-parser-functions.spicy:17:8-17:12", Null); ((*self).x, __cur) = (*unpack<uint<8>>((__cur, hilti::ByteOrder::Network))); if ( __trim ) (*__data).trim(begin(__cur)); # End parsing production: Variable: x -> uint<8> (*self).__error = __error; __error = (*self).__error; }"
+[debug/optimizer] [<no location>] statement::Try "try { # "<...>/default-parser-functions.spicy:18:8-18:12" # Begin parsing production: Variable: y -> uint<8> spicy_rt::waitForInput(__data, __cur, 1, "expecting 1 bytes for unpacking value", "<...>/default-parser-functions.spicy:18:8-18:12", Null); ((*self).y, __cur) = (*unpack<uint<8>>((__cur, hilti::ByteOrder::Network))); if ( __trim ) (*__data).trim(begin(__cur)); # End parsing production: Variable: y -> uint<8> (*self).__error = __error; (*self).__on_y((*self).y); __error = (*self).__error; } catch ( hilti::SystemException __except ) { throw; }" -> replacing rethrowing try/catch with just the block
+[debug/optimizer] [<no location>] statement::Try "try { # "<...>/default-parser-functions.spicy:18:8-18:12" # Begin parsing production: Variable: y -> uint<8> spicy_rt::waitForInput(__data, __cur, 1, "expecting 1 bytes for unpacking value", "<...>/default-parser-functions.spicy:18:8-18:12", Null); ((*self).y, __cur) = (*unpack<uint<8>>((__cur, hilti::ByteOrder::Network))); if ( __trim ) (*__data).trim(begin(__cur)); # End parsing production: Variable: y -> uint<8> (*self).__error = __error; (*self).__on_y((*self).y); __error = (*self).__error; } catch ( hilti::SystemException __except ) { throw; }" -> statement::Block "{ # "<...>/default-parser-functions.spicy:18:8-18:12" # Begin parsing production: Variable: y -> uint<8> spicy_rt::waitForInput(__data, __cur, 1, "expecting 1 bytes for unpacking value", "<...>/default-parser-functions.spicy:18:8-18:12", Null); ((*self).y, __cur) = (*unpack<uint<8>>((__cur, hilti::ByteOrder::Network))); if ( __trim ) (*__data).trim(begin(__cur)); # End parsing production: Variable: y -> uint<8> (*self).__error = __error; (*self).__on_y((*self).y); __error = (*self).__error; }"
 [debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); 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 ) { throw; }" -> replacing rethrowing try/catch with just the block
 [debug/optimizer] [<no location>] statement::Try "try { hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); 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 ) { throw; }" -> statement::Block "{ hilti::debugIndent("spicy"); local iterator<stream> __begin = begin(__cur); local strong_ref<stream> filtered = Null; if ( ! filtered ) __result = (*self).__parse_foo__P1_stage2(__data, __begin, __cur, __trim, __lah, __lahe, __error); }"
 [debug/optimizer] [default-parser-functions.spicy:10:1-21:2] declaration::Constant "const bool __feat%foo@@P0%supports_filters = False;" -> disabled feature 'supports_filters' of type 'foo::P0' since it is not used
@@ -370,8 +374,14 @@
 [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] declaration::Field "hook void __on_x_error(string __excpt);" -> 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] operator_::struct_::MemberCall "(*self).__on_x_error(hilti::exception_what(__except))" -> 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] [default-parser-functions.spicy:17:5-17:13] statement::Expression "default<void>();" -> removing default<void> statement
+[debug/optimizer] [default-parser-functions.spicy:18:5-18:15] declaration::Field "hook void __on_y_error(string __excpt);" -> null
+[debug/optimizer] [default-parser-functions.spicy:18:5-18:15] operator_::struct_::MemberCall "(*self).__on_y_error(hilti::exception_what(__except))" -> expression::Ctor "default<void>()" (replacing call to unimplemented method with default value)
+[debug/optimizer] [default-parser-functions.spicy:18:5-18:15] 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
@@ -416,6 +426,8 @@
 [debug/optimizer] removing field for unused method foo::P2::__on_0x25_synced
 [debug/optimizer] removing field for unused method foo::P2::__on_0x25_undelivered
 [debug/optimizer] removing field for unused method foo::P2::__on_x
+[debug/optimizer] removing field for unused method foo::P2::__on_x_error
+[debug/optimizer] removing field for unused method foo::P2::__on_y_error
 [debug/optimizer] removing field for unused method foo::P2::__str__
 [debug/optimizer] removing field for unused method struct_3::parse1
 [debug/optimizer] removing field for unused method struct_3::parse2