supports_filter optimization doesn't kick in

[rsmmr]

module Test;

public type Foo = unit {
    bar: Bar;
};

type Bar = unit {
    x: uint32;
    y: uint32;
};

No filters in use, but we still generate this code:

method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::RecoverableFailure>> Test::Bar::__parse_stage1(inout value_ref<stream> __data, copy view<stream> __cur,>
[...]
            if ( filtered = spicy_rt::filter_init(self, __data, __cur) ) {
                local value_ref<stream> filtered_data = filtered;
                (*self).__parse_Test_Bar_2_stage2(filtered_data, (*filtered_data), __trim, __lah, __lahe, __error);
                __cur = __cur.advance(|__cur|);

                if ( __trim )
                    (*__data).trim(begin(__cur));

                __result = (__cur, __lah, __lahe, __error);
            }



        if ( ! filtered )
            __result = (*self).__parse_Test_Bar_2_stage2(__data, __cur, __trim, __lah, __lahe, __error);

    }
[...]
}

In fact, if I remember right, originally (i.e., before we switched to automatic optimization), we wouldn't be generating the stage1 function at all if the unit didn't need to support filtering.

[rsmmr]

For testing purposes, here's patch that generally disables filter support: https://gist.github.com/rsmmr/38aa07d5adc50381513f4fbccf08f2ce

[rsmmr]

We should probably also double check other optimizations if they work as intended.

[bbannier]

I checked the code generated from a version before we added optimizations (v1.1.0), and it looks like even then we would have emitted the filter_init block and the stage1 functions. I'll check why we are able to remove filter support for Foo, but not Bar.

[bbannier]

The issue here seems to be that we unconditionally accessBar's __filters field. That direct access is not guarded by a feature flag (feature flag-dependent conditional) while the field documents that access to it requires support for filters. We access filters for pretty much all low-level data extraction from the stream, see users of _filters .

This was not an issue for Foo since its parser forwarded completely to Bar,

method method tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::RecoverableFailure>> Test::Foo::__parse_Test_Foo_stage2(inout value_ref<stream> __data, copy view<stream> __cur, copy bool __trim, copy int<64> __lah, copy iterator<stream> __lahe, copy optional<hilti::RecoverableFailure> __error) {
    # "/tmp/bla.spicy:3:19-5:2"
    local tuple<view<stream>, int<64>, iterator<stream>, optional<hilti::RecoverableFailure>> __result;
    # "/tmp/bla.spicy:7:12-10:2"

    # Begin parsing production: Unit: Test_Bar -> x y
    (*self).bar = default<Bar>();
    (__cur, __lah, __lahe, __error) = (*(*self).bar).__parse_stage1(__data, __cur, __trim, __lah, __lahe, __error);
    # End parsing production: Unit: Test_Bar -> x y
    ...

Adding full support for removing filters would roughly look like this:

• modify _filters so that it does not return a unit's __filters directly anymore, but instead a ternary __feat%Foo_Bar%supports_filters ? <direct field access> : <null value> so the code can be constant-folded by the optimizer
  • adapt the code detecting feature-dependent code under conditionals so it also understands the ternary operator
• annote member functions triggering filter use (probably only connect_filter for units and Sinks) with &requires-type-feature="supports_filters"; this requires support for feature requirements on the implicit method self parameter
  • add support for method self parameter attributes to Signature
  • add optimizer support to extract feature requirements from method self parameter attributes
• if with that the optimizer is not able to disable filter support with that due to deep call chains:
  • remove feature requirement on functions which currently require supports_filters; in principle it should be possible to drive this entirely via the requirement on the __filters member

[bbannier]

Fortunately this turned out to be easier to implement than I have anticipated since I didn't to implement requirements on self parameter (the implementation of the member function connect_filter just forwarded to the non-member filter_connect which was already annotated)

The only piece breaking this previously was the unguarded access to the __filters unit member which I added in #1404.

It also turned out we already had above reproducer in our existing baselines. With above patch we now remove filter support as well in that case (in addition to all other optional parser features we already removed previously).

[bbannier]

Also backported to release/1.5.