search

arr-ai / arrai

The ultimate data engine.
http://arr.ai
Apache License 2.0
20 stars 15 forks source link

Fix precedence of safe-accessor tail #525

Open marcelocantos opened 4 years ago

marcelocantos commented 4 years ago

Fixes #552

Change safe-accessor tail to expect an expr instead of the next rule on the precedence stack.

The expression a.b?:c.d used to parse as (a.b?:c).d. This is confusing because it means that .d, which looks like it belongs to c, actually applies to a.b if a has a b attribute. This PR changes it to parse as a.b?:(c.d).

This is actually a bit of a hack. My preference would be to loop back to the current rule. However, wbnf doesn't have a way to refer to the current rule, only the next rule (@) or back to the top (expr). It might be better to add this capability to wbnf. Comments welcome.

Checklist:

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 8d2f7b7b1dc05aee43438caf9f604d54f9184dc3-PR-525

Files with Coverage Reduction New Missed Lines %
rel/value_set_generic.go 2 74.47%
<!-- Total: 2 -->
Totals Coverage Status
Change from base Build cea02743513c00478f8b88caad0546ce96ae2133: 0.0%
Covered Lines: 4533
Relevant Lines: 9502
💛 - Coveralls
orlade-anz commented 4 years ago

Can you add a description of what this is actually doing for posterity?

It looks vaguely related to https://github.com/arr-ai/arrai/issues/532 but doesn't fix it.

marcelocantos commented 4 years ago

We really should fix the stack operator in wbnf so that this rule can be self-referential. https://github.com/arr-ai/wbnf/issues/85