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

Parser should discard literal suffix in primitiveValue #125

Closed
Gabriel-Darbord opened this issue Oct 27, 2023 · 6 comments
Closed

Parser should discard literal suffix in primitiveValue #125

Gabriel-Darbord opened this issue Oct 27, 2023 · 6 comments

Comments

@Gabriel-Darbord
Copy link
Member

When parsing a literal value with a suffix (like 2L or 2.0f), the parser should use the suffix to create the correct entity, but it should not be included in the primitiveValue field.
The following:

JavaSmaCCProgramNodeImporterVisitor parseCodeMethodString: 'int f() { return 2.0f; }'

currently gives the primitiveValue 2.0f, but it should give 2.0.
The suffix logic is already implemented in the exporter, resulting in double suffixes, which is an error.

@NicolasAnquetil
Copy link
Contributor

I agree that it should create the correct entity ;-)
But I would rather keep the suffix in the primitiveValue and remove it from the exporter

The idea is keeping the model as close to the source code as possible without making too much assumptions on how Java interpret things

@NicolasAnquetil
Copy link
Contributor

And actually, for FASTJavaLongLiteral, it is never created by the Importer, so it is dead code

@Gabriel-Darbord
Copy link
Member Author

I agree that it should create the correct entity ;-)

The parser does not do that at the moment :/ (#124 and #126)

And actually, for FASTJavaLongLiteral, it is never created by the Importer, so it is dead code

Where is the dead code? I think it is missing code.
The importer is missing visitLongLiteral:, and the parser is missing some logic to find the corresponding primitive type of literals.

@NicolasAnquetil
Copy link
Contributor

Still not convince this belongs in the parser or that we should have a special AST node to differentiate integers from longs (or floats from doubles).
It brings extra work and I don't see the benefit

@Gabriel-Darbord
Copy link
Member Author

JDT only uses NumberLiteral.
I agree with having either a node for each number literal type or one for all, but no in-between as it is now.
So far I wanted to do the first one for dispatch, but there's not much to dispatch if not for the suffix handling.

@NicolasAnquetil
Copy link
Contributor

Agree with the idea of only one NumberLiteral I am closing this issue and creating a new one (#157)

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

No branches or pull requests

2 participants