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.

Sign up for GitHub

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

Jump to bottom

Class ParseTreeProperty #7

Open
HermanPeeren opened this issue Sep 8, 2020 · 4 comments · May be fixed by #8
Open

Class ParseTreeProperty #7

HermanPeeren opened this issue Sep 8, 2020 · 4 comments · May be fixed by #8
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@HermanPeeren
Copy link

In the API for the runtime, as documented for Java in https://www.antlr.org/api/Java, we have a Class ParseTreeProperty in the Tree-package. It is to easily attach a value to a node in the parse tree. Java uses an IdentityHashMap for that, with the node / parse tree as key. In PHP we don't have such a datastructure, where we can use an object as key. That is probably why you left this class out in the PHP runtime?

My thoughts:

  • to make it compatible with the general runtime API, we can implement another way to associate values with a node. Maybe using spl_object_id or spl_object_hash as a key/identity for the node.
  • or do it the dirty PHP-way, where we can add values to any object: adding values directly to the node. Disadvantage: you change the tree, which might give problems if you want to reuse the tree for multiple applications (multiple listeners / visitors using the same tree).
    My preference is option nr. 1, as it conforms the general API. Shall I make a proof of concept?

Maybe I'm overlooking something here and is this a non-issue... Please let me know your thoughts.
@marcospassos
Copy link
Collaborator

I left this implementation out because it's just a utility map class, and PHP provides native support for that. I believe that SplObjectStorate has a perfect fit for this use case, or I am missing something?

The only benefit I see in shipping it as part of the runtime is making it easier for people coming from other targets to reproduce the tutorials out there (a new section in the docs may work as well). In that sense, we can implement the PHP version of ParseTreeProperty on top of the SplObjectStorate.

What your thoughts?

@marcospassos marcospassos added enhancement New feature or request question Further information is requested labels Sep 8, 2020
@HermanPeeren
Copy link
Author

O, perfect, didn't know SplObjectStorage ::blushblush::, but that is exactly it. I'd prefer to wrap that in a ParseTreeProperty-class, so people can easily use it when reading material about ANTLR that is not specifically for PHP, like section 7.5 inTerrence Parr's book (about Sharing Information Among Event Methods).

@marcospassos
Copy link
Collaborator

Contributions are welcome! Feel free to open a PR :)

@marcospassos marcospassos added help wanted Extra attention is needed and removed question Further information is requested labels Sep 8, 2020
@HermanPeeren HermanPeeren linked a pull request Sep 9, 2020 that will close this issue
Open
@HermanPeeren
Copy link
Author

When no value for a node is stored, null is returned.
Standard SplObjectStorage::offsetGet() would throw an Exception there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add ParseTreeProperty class HermanPeeren/antlr-php-runtime
2 participants
@HermanPeeren @marcospassos