Cleanup the ParseTree hierarchy and open its functions #194
Conversation
lppedd
force-pushed
the
refactor/parsetree
branch
from
404b7c0
to
5e2e82e
Compare
| val start = this.start | ||
| val stop = this.stop | ||
|
|
||
| if (start != null && stop != null) { |
There was a problem hiding this comment.
Perhaps we could write this as `if (start != null && stop?.endPoint() != null) {
There was a problem hiding this comment.
Nope, because you'd have to assign the result of stop?.endPoint() to a variable x anyway, to assert x != null to be able to pass it to Position(start.startPoint(), x). So stop?.endPoint() != null is a redundant call to endPoint().
Another argument for calling startPoint() and endPoint() only once is they're functions, and the result may vary between invocations.
| /** | ||
| * If we are debugging or building a parse tree for a visitor, | ||
| * we need to track all the tokens and rule invocations associated | ||
| * with this rule's context. This is empty for parsing w/o tree constr. | ||
| * operation because we don't have the need to track the details about | ||
| * how we parse this rule. | ||
| */ | ||
| @JvmField |
There was a problem hiding this comment.
Why do we need to make this a JvmField? Do we want the outside world to be able to set the field directly?
There was a problem hiding this comment.
Well, the outside world was able to set that property anyway, even without @JvmField. The difference is without the annotation a getter, a setter, and a backing private field would be generated. With the annotation we only have the public backing field.
Applying @JvmField aligns the code with the Java runtime.
| // The original condition was 'currentNode != null', | ||
| // but since nodeStack.removeFirst() throws if there | ||
| // is no element, the condition is always true | ||
| } while (true) |
|
This one has been merged. Should we release 1.0.0 ? |
|
@ftomassetti I'd like to make the identity map and weak map internal, as consumers should not use them. 1 hour and I'll open the PR. |
For the historical reason that
RuleContext.parentwasn't marked with@JvmField, the two original JavagetParent()andsetParent()functions had to be renamed toreadParent()andassignParent()to avoid a collission with the JVM-generated getter and setter for theparentproperty.This change encapsulates the
parentfield into theRuleContexthierarchy - marking it with@JvmFieldand hiding it from the outer world - and renames the get/set functions to their original names.ParserRuleContext's functions have also been opened.