search

Strumenta / antlr-kotlin

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

Complete code cleanup for 1.0.0 #188

Open lppedd opened 1 month ago

lppedd commented 1 month ago

This refactor completes the work needed for 1.0.0 release.

lppedd commented 1 month ago

@ftomassetti the last commit is important when using the contextSuperClass option.
I was trying to use it and found myself working with a mostly closed ParserRuleContext.

lppedd commented 1 month ago

@ftomassetti btw, I've been also working with ParseTreeProperty, which uses IdentityHashMap under the hood.

In JS IdentityHashMap is implemented as an HashSet, so there might be instances where putting and then getting doesn't work as expected. I will provide a proper implementation for the JS source set soon. For the rest of the targets (WASM, Native) it will still be problematic until I fix them, but currently there is no clean way to do it.

Maybe it's worth adding a warning in the readme about ParseTreeProperty and IdentityHashMap?

Currently the KMP IdentityHashMap works because the entire ParseTree hierarchy doesn't implement hashCode(), and thus this happens:

Target What does it use?
JVM identityHashCode
JS memoized random hash code per instance (like identityHashCode)
Native identityHashCode
WASM identityHashCode
lppedd commented 1 month ago

I want to clarify that the warning on the readme is for the 1.0.0 release.

The implementation will be out with a 1.0.x, if it's ok for you.

lppedd commented 1 month ago

I think I will break up this PR into multiple PRs, I feel like it's too big. I'll make this one a draft, don't merge it 😄