Guidance on using the Matcher and syntaxTreeContext classes

[Omar Ahmed]

Greetings all,

I was a little bit stuck in trying to match a kTernaryExpressionNode inside another kExpressionNode, so how could I do that?

Also I wanted to ask is there a document that illustrates how to use the Matcher and syntaxTreeContext classes properly or something like a doxygen document that says how to uses the API?

Thanks in advance!

Omar Ahmed

[David Fang]

I have a little documentation at https://github.com/google/verible/blob/master/doc/style_lint.md (mirror).  That gives a higher level overview with links into the class definitions, with more detailed docstrings.  However, before recommending going down the route of using matchers, can you explain what you're trying to do first?  Is this referring to a particular issue on github?

--
You received this message because you are subscribed to the Google Groups "Verible Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to verible-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/verible-dev/eaa32508-3463-484f-a48e-3da1b7580da5n%40googlegroups.com.

[Omar Ahmed]

I was working on this issue: https://github.com/google/verible/issues/383

I was trying to suggest parentheses around ternary expression in some cases:

- like if we have:" assign a = condition_a? 0: 1 + b ", I think this case would require parentheses around the ternary expression.

- or we have " assign a = condition_a? condition_b? 0 : 1 : 1", this is the example suggested in the issue.

So I came with two ideas which I was trying to implement:

- Check if the ternary expression is a child of an expression node then list all the expression types in a switch statement for checking and ofc the ternary expression doesn't have a parentheses leaf nodes as siblings to it, in this case we would suggest parentheses around the ternary expression in some case types of expressions

- check only if the ternary expression is the highest level of expression and it doesn't have a sibling expressions like a + or - expression. if it doesn't have then it is okay, else we check if it have parentheses leaf nodes as siblings then it is okay else if it doesn't have parentheses leaf nodes as siblings and it have a sibling of another expression node then we will suggest parentheses around it.

Examples for the second approach:

assign a = condition_a? 0: 1       // we will not suggest in this case

assign a = condition_a? 0: 1 + b            // we will suggest in this case

assign a = condition_a? 0: 1 + condition_a? 0: 1       // we will suggest in this case

[David Fang]

Replies inline:

On Mon, Oct 26, 2020 at 8:33 AM Omar Ahmed <omarpir...@gmail.com> wrote:

I was working on this issue: https://github.com/google/verible/issues/383

I was trying to suggest parentheses around ternary expression in some cases:

- like if we have:" assign a = condition_a? 0: 1 + b ", I think this case would require parentheses around the ternary expression.

I'm not entirely sure here because the ternary operator has a lower precedence than arithmetic.  As you've written it, the parser interprets it as "condition_a? 0: (1 + b)".  Now whether users would like to see that explicitly parenthesized is up to debate.

 

- or we have " assign a = condition_a? condition_b? 0 : 1 : 1", this is the example suggested in the issue.

As the referenced guide suggests, this one does want parentheses, even if the parser already interprets this according to precedence rules.  I suggest we focus on this case only for now, and generalize later.

 

So I came with two ideas which I was trying to implement:

- Check if the ternary expression is a child of an expression node then list all the expression types in a switch statement for checking and ofc the ternary expression doesn't have a parentheses leaf nodes as siblings to it, in this case we would suggest parentheses around the ternary expression in some case types of expressions

- check only if the ternary expression is the highest level of expression and it doesn't have a sibling expressions like a + or - expression. if it doesn't have then it is okay, else we check if it have parentheses leaf nodes as siblings then it is okay else if it doesn't have parentheses leaf nodes as siblings and it have a sibling of another expression node then we will suggest parentheses around it.

Examples for the second approach:

assign a = condition_a? 0: 1       // we will not suggest in this case

assign a = condition_a? 0: 1 + b            // we will suggest in this case

assign a = condition_a? 0: 1 + condition_a? 0: 1       // we will suggest in this case

For this task, I would suggest adding some helper functions to CST/expression.h:

// Returns the Symbol directly underneath a `kExpression` node, otherwise returns itself.

const verible::Symbol* UnwrapExpression(const verible::Symbol* expression);

// Returns the predicate expression of a kTernaryExpression

const verible::Symbol* GetConditionExpressionPredicate(const verible::Symbol* ternary_expression);

// Returns the true-case expression of a kTernaryExpression

const verible::Symbol* GetConditionExpressionTrueCase(const verible::Symbol* ternary_expression);

// Returns the false-case expression of a kTernaryExpression

const verible::Symbol* GetConditionExpressionFalseCase(const verible::Symbol* ternary_expression);

I also wouldn't mind globally replacing kTernaryExpression with kConditionExpression.  :)

In the lint rule, instead of trying to use a matcher, implement an override for SyntaxTreeLintRule::HandleNode, and switch-case on node.tag, for kTernaryExpression, and examine from there.

 

On Monday, 26 October 2020 at 04:35:07 UTC+2 David Fang wrote:

I have a little documentation at https://github.com/google/verible/blob/master/doc/style_lint.md (mirror).  That gives a higher level overview with links into the class definitions, with more detailed docstrings.  However, before recommending going down the route of using matchers, can you explain what you're trying to do first?  Is this referring to a particular issue on github?

On Sun, Oct 25, 2020 at 3:23 PM Omar Ahmed <omarpir...@gmail.com> wrote:

Greetings all,

I was a little bit stuck in trying to match a kTernaryExpressionNode inside another kExpressionNode, so how could I do that?

Also I wanted to ask is there a document that illustrates how to use the Matcher and syntaxTreeContext classes properly or something like a doxygen document that says how to uses the API?

Thanks in advance!

Omar Ahmed

--
You received this message because you are subscribed to the Google Groups "Verible Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to verible-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/verible-dev/eaa32508-3463-484f-a48e-3da1b7580da5n%40googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "Verible Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to verible-dev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/verible-dev/7faea37d-d9ce-4d87-9cbb-658b5830bb82n%40googlegroups.com.

[Omar Ahmed]

On Monday, 26 October 2020 at 22:42:17 UTC+2 David Fang wrote:

Replies inline:

On Mon, Oct 26, 2020 at 8:33 AM Omar Ahmed <omarpir...@gmail.com> wrote:

I'm not entirely sure here because the ternary operator has a lower precedence than arithmetic.  As you've written it, the parser interprets it as "condition_a? 0: (1 + b)".  Now whether users would like to see that explicitly parenthesized is up to debate.

As the referenced guide suggests, this one does want parentheses, even if the parser already interprets this according to precedence rules.  I suggest we focus on this case only for now, and generalize later.

 I was trying to suggest parentheses around this cases to eliminate confussion for the developer of the addition is inside the ternary exression or outside. but I think you are right let's keep those for later and I will work on the obvious case for now and save that for later.

 

For this task, I would suggest adding some helper functions to CST/expression.h:

// Returns the Symbol directly underneath a `kExpression` node, otherwise returns itself.

const verible::Symbol* UnwrapExpression(const verible::Symbol* expression);

// Returns the predicate expression of a kTernaryExpression

const verible::Symbol* GetConditionExpressionPredicate(const verible::Symbol* ternary_expression);

// Returns the true-case expression of a kTernaryExpression

const verible::Symbol* GetConditionExpressionTrueCase(const verible::Symbol* ternary_expression);

// Returns the false-case expression of a kTernaryExpression

const verible::Symbol* GetConditionExpressionFalseCase(const verible::Symbol* ternary_expression);

I also wouldn't mind globally replacing kTernaryExpression with kConditionExpression.  :)

In the lint rule, instead of trying to use a matcher, implement an override for SyntaxTreeLintRule::HandleNode, and switch-case on node.tag, for kTernaryExpression, and examine from there.

 This suggestions and the helper functions suggestion are great. Now, the task is a lot more clear for me. Thank you :)