Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for optional getters #1576

Merged
merged 2 commits into from
Jan 4, 2017
Merged

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Jan 4, 2017

This analysis is required for proper code generation in the TypeScript target when strict null checks are enabled. It also applies to targets intending to differentiate optional values from required values.

This analysis is required for proper code generation in the TypeScript target
when strict null checks are enabled. It also applies to targets intending to
differentiate optional values from required values.
@parrt
Copy link
Member

parrt commented Jan 4, 2017

Looks interesting. do we have a few examples that exercise this stuff? i guess we have one issue you just resolved, right?

@sharwell
Copy link
Member Author

sharwell commented Jan 4, 2017

@parrt I used this to fix tunnelvisionlabs/antlr4ts#265. That issue includes an example which highlights the difference in the generated code produced by this target.

I also mentioned optional values. In a hypothetical modified Java target, one could use Optional<TContext> instead of TContext for the return value of optional accessors. In such a target, all generated accessors would have the property that the return value is never null. Perhaps it's not a great idea for performance reasons, but I think it serves as an interesting example.

@sharwell
Copy link
Member Author

sharwell commented Jan 4, 2017

@parrt The Swift target may also benefit from this change, but I don't know enough about its handling of nullable types to say for sure. It does appear that the ContextTokenGetterDecl and ContextRuleGetterDecl templates in that target currently both assume that the result is nullable.

@@ -134,6 +183,11 @@ protected void exitSubrule(GrammarAST tree) {
for (Map.Entry<String, MutableInt> entry : frequencies.peek().entrySet()) {
entry.getValue().v = 2;
}

int multiplier = tree.getType() == POSITIVE_CLOSURE ? 1 : 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we change this to a boolean rather than a multiplier? ;) Ain't that fortran66 style hahah

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simplified this even more 😄

@parrt parrt added this to the 4.6.1 milestone Jan 4, 2017
@parrt parrt merged commit a17b299 into antlr:master Jan 4, 2017
@sharwell sharwell deleted the optional-getters branch January 4, 2017 17:52
@parrt
Copy link
Member

parrt commented Jan 4, 2017

@janyou @hanjoes might be interesting to Swift target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants