search

Strumenta / antlr-kotlin

Support for Kotlin as a target for ANTLR 4
Apache License 2.0
221 stars 47 forks source link

Avoid NPE when dealing with visitors #183

Closed lppedd closed 2 months ago

lppedd commented 2 months ago

This is the idea in practice.

Fixes #182

lppedd commented 2 months ago

@ftomassetti I don't have better ideas, and this looks like it's good enough, so I marked the PR as reviewable.

ftomassetti commented 2 months ago

I also do not have better ideas and I did not use visitors in many years. The only comment I can make is that a test for this specific use case would be a nice to have, but I would not consider a requirement for merging this. If I do not hear from you I will just go ahead and merge it on Friday

lppedd commented 2 months ago

@ftomassetti added a very simple test case, I think it's sufficient to establish if everything works well together.

Next would be RC4? If everything is ok after a couple weeks, you could release 1.0.0. No point in waiting more time.

ftomassetti commented 2 months ago

Thank you @lppedd

Next would be RC4? If everything is ok after a couple weeks, you could release 1.0.0. No point in waiting more time.

Yes and yes