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

Rename & fix or delete 'InterTypeClassHeaderName1' grammar rule #208

Open
kriegaex opened this issue Jan 4, 2023 · 3 comments
Open

Rename & fix or delete 'InterTypeClassHeaderName1' grammar rule #208

kriegaex opened this issue Jan 4, 2023 · 3 comments
Assignees
Labels
JDT Core Eclipse Java Development Tools (AspectJ fork)

Comments

@kriegaex
Copy link
Contributor

kriegaex commented Jan 4, 2023

When fixing another grammar problem, I noticed Jikespg output as follows:

The following Non-Terminal is useless: InterTypeClassHeaderName1

Therefore, I added a TODO comment in eclipse-aspectj/eclipse.jdt.core@f2a47a3, see also directly in the file:

-- TODO: Rename 'InterType*' to 'Intertype*'? Without renaming, Jikespg says:
--   The following Non-Terminal is useless: InterTypeClassHeaderName1
-- But after renaming, it says instead:
--   Reduce/reduce conflict on "LESS" between rule 410 and 303
--   Reduce/reduce conflict on "LESS" between rule 405 and 304
InterTypeClassHeaderName1 ::= Modifiersopt 'class' OnType TypeParametersAsReference '.' JavaIdentifier
/.$putCase consumeIntertypeClassHeaderName(true); $break ./
/:$readableName IntertypeClassHeader:/

@aclement, can you please review this portion of the grammar and tell me what is supposed to happen there? Is that rule (production) superfluous or supposed to be fixed?

@kriegaex kriegaex added the JDT Core Eclipse Java Development Tools (AspectJ fork) label Jan 4, 2023
@aclement
Copy link
Contributor

aclement commented Jan 4, 2023

I'm afraid I don't speak jikespg. That area of the project is extremely fragile as since the original work X number of years ago (not by me), it has not been touched other than in emergencies and then we rely on (a) can we get a valid file out (whether there are warnings or not) and (b) does it pass all the tests. Another reason to switch to plain java with annotations and away from JDT fork.

@kriegaex
Copy link
Contributor Author

kriegaex commented Jan 5, 2023

@aclement: Well, if you simply look at the grammar rules, you notice that due to upper vs. lower case spelling (typo?) in the commented rule, it is not being referenced elsewhere. The rule in question is different from the preceding one in that it caters to generics, i.e. something like ITDs like public class XY<T>.foo, while the preceding rule IntertypeClassHeaderName1 (lower-case "t" in "Intertype") seems to cover the non-generic case public class XY.foo. I.e., there might be something not working which is supposed to syntax-wise. I thought that maybe you could shed light on this issue.

@aclement
Copy link
Contributor

aclement commented Jan 5, 2023

It sounds like you are correct. I don't recall. As I say, the tests are the usual arbiter on whether things are working. We might well not have all the tests we need around generics and ITDs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JDT Core Eclipse Java Development Tools (AspectJ fork)
Projects
None yet
Development

No branches or pull requests

2 participants