[antlr-interest] Bug or misunderstanding?: missing attribute access on rule scope

[Kaleb Pederson]

On Fri, Oct 16, 2009 at 7:08 PM, John B. Brodie <jbb at acm.org> wrote:
> On Fri Oct 16 12:21:33 PDT 2009, Kaleb Pederson asked:
>> I'm getting an error that doesn't make any sense, either because I've
>> missed something fundamental or I've stumbled across a bug. I'm doing
>> some type checking within a tree parser.  I have a plusMinusExpression
>> which can either be a negation or a subtraction expression.  In order
>> to to check to see which it is, I have `if ($rhs != null)` within my
>> action.  That line, however, causes the following error:
>>
>> SemanticChecker.g:163:3: missing attribute access on rule scope: rhs
>>
>> The grammar fragment follows:
>>
>> plusMinusExpression returns [Type type]
>>       :       ^(PLUS lhs=baseExpression rhs=baseExpression) { /*...*/ }
>>               |       ^(MINUS lhs=baseExpression rhs=baseExpression?)
>>               {
>>                       typeChecker.assertIsNumericType($lhs.type);
>>                       if ($rhs != null)
>>                       {
>>                               typeChecker.assertIsNumericType($rhs.type);
>>                               typeChecker.assertEqualTypes($lhs.type, $rhs.type);
>>                       }
>>                       $type = $lhs.type;
>>               }
>>       ;
>
> Others in this thread have pointed out that you should use ($rhs.tree != null) in your if statement.
>
> I would like to, respectfully, suggest that you are doing the Wrong Thing(TM) in your action code.
> And it is wrong on two levels - the first mis-use is rather shallow and the second is a rather more
> deeper mis-use.
>
> I mean nothing personal nor pejorative by these comments. I am just trying to point out a couple
> of "better" ways to do the above. And of course "better" is a qualitative assessment unique to each
> reader of this message (e.g. what is "better" to me may not be "better" to you...).

Understood. No offense taken :).

> The first, shallow, mis-use (IMHO) is to not simply let the Tree Walker code decide for you whether
> or not the rhs tree node is present. That is, instead of
>
>   | ^(MINUS lhs=baseExpression rhs=baseExpression?) { /* figure out what to do */ }
>
> do something like this:
>
>   | ^(MINUS lhs=baseExpression) { /* handle the 1-arity '-' operator here */ }
>   | ^(MINUS lhs=baseExpression rhs=baseExpression) { /* handle the 2-arity subtraction here */ }
>
> e.g. the Tree Walker code has to already poke around in your tree in order to determine whether or
> not the rhs is present (as part of its tree pattern matching) --- so let it control the invocation
> of your actions.

Good suggestion.

> The second, i think deeper, mis-use is that you have (apparently) discarded information discovered
> during parsing and AST creation that you could use to guide your subsequent AST processing.
>
> Surely in your Parser the 1-arity '-' appears in a different syntactic context from the 2-arity '-'
> operator ---- so when parsing "a+-b" we know that the '-' must be the 1-arity operator, but when we
> parse "a-b" we know that the '-' must be the 2-arity operator.
>
> So I suggest to you that your Parser should differentiate the MINUS tree --- since, I assert, that
> it already does so as part of the parse --- by creating a different (with imaginary root) node for
> the 1-arity case. So "a+-b" obtains the tree (PLUS a (NEGATE b)) and "a-b" obtains the tree
> (MINUS a b) or something similar in accord with your taste...
>
> And so, with this approach, in all of your post-processing of the AST, the 1-arity and 2-arity
> operations have already been differentiated in the tree itself and therefore can be directly
> determined --- no need to noodle around about whether or not 1 or 2 children are present.

Very true. I know during the parse exactly which one it is, and I
intentionally created the AST the way it is, but inadvertently made it
harder for myself :(.  From now on, I'll  "always remember to keep
information available in the initial parse for later passes."

I was hoping to subvert some of these problems / poor decisions by
asking about best practices for AST building
(http://www.antlr.org/pipermail/antlr-interest/2009-October/036238.html),
but nobody responded at the time.

> and there is also the possibility of having ONLY the 2-arity '-' operator because in Algebra
> the 1-arity '-' is an implicit 2-arity operation involving zero. so the "a+-b" would become
> (PLUS a (MINUS 0 b)) --- but this just tends to break my brain so i do not do that..... ;-)
>
> and, please, can we just not discuss the whole treatment of 2-arity '-' as an addition
> with multiply by -1 ..... my brain tends to just shutdown at that prospect ;-)

I think both of these are worth considering in the context of the
target language / architecture.

Thank you for the suggestions, I appreciate the advise.

--
Kaleb Pederson

Twitter - http://twitter.com/kalebpederson
Blog - http://kalebpederson.com