Page MenuHomePhabricator

Relax the restriction that parsed arguments must match op arguments in ODS.
Needs ReviewPublic

Authored by Mokosha on Oct 19 2020, 5:17 PM.

Details

Summary

In particular, when an op defines one or more optional attributes, we should
be able to write pattern rewrites that omit those attributes.

Diff Detail

Event Timeline

Mokosha created this revision.Oct 19 2020, 5:17 PM
Mokosha requested review of this revision.Oct 19 2020, 5:17 PM

Could you please update the documentation and description of the change?

Seems like you are assuming some C++ like default call behavior from the patterns, or how do you plan to generalize this?

And some tests for error cases would be good.

Mokosha updated this revision to Diff 299436.Oct 20 2020, 12:06 PM

Fix clang-format to use camel case instead of snake case.

Mokosha updated this revision to Diff 304587.Wed, Nov 11, 10:55 AM

Fix cases where we may have location directives

Mokosha updated this revision to Diff 304679.Wed, Nov 11, 4:21 PM

Fix some tests to use actual required args and add failure tests

Mokosha updated this revision to Diff 304694.Wed, Nov 11, 5:58 PM

Add some documentation and refine the tests a bit further.

I've added some documentation that hopefully highlights the intent behind the change. The way that we generate rewrite implementations for ops (i.e. the build() call that we use) means that we can omit some of the attributes if they're marked optional and everything should "just work". The tests that were added will hopefully catch this if anything fails in the future. PTAL (and let me know if I'm missing something egregious here!)