eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
157 stars 125 forks source link

Annotation processing with 'querydsl-apt' leads to compiler error #565

Closed kriegaex closed 1 month ago

kriegaex commented 1 year ago

This issue was originally brought up in https://github.com/dev-aspectj/aspectj-maven-plugin/issues/108, then investigated in https://github.com/eclipse/org.aspectj/issues/195, where I established that it is not an AspectJ Compiler (AJC) error but actually an upstream issue here in JDT Core (tested with stand-alone ECJ). Please read the AspectJ issue for more details and a stack trace swallowed by ECJ/AJC.

A Maven reproducer for AspectJ is at https://github.com/kriegaex/JDTCoreAPTBug, a shell script reproducing the problem with ECJ is here. Please run the Maven build with mvn clean verify first in order to make sure to have all the dependencies referred to in the shell script in your local Maven repo. By changing property ecj.version in the POM, you can also make sure that a different (e.g. more recent) ECJ version will be downloaded. Then please adjust the MVN_REPO shell variable in the script according to your local path.

Feel free to ask questions.

kriegaex commented 1 year ago

@sravanlakkimsetti, @jarthana, @mpalat: Sorry to randomly directly address names which I associate with JDT Core development, but I would like to get some feedback on this APT bug. Can you reproduce it? Feel free to dispatch to other team members.

zenbones commented 1 year ago

Just checking back into my open tickets. This is clearly not a priority which I get, but having anyone look into this, just to get it on the radar, would be nice.

kriegaex commented 1 year ago

It has been 9 months without any feedback for this reproducible bug. Is there anything missing you need to get motivated to look into it? Is my reproducer GitHub project somehow inadequate?

kriegaex commented 7 months ago

I pushed a commit with version bumps to the reproducer repository. With ECJ 3.36.0, the problem still persists, and I guess it is not going to solve itself. It is going on 15 months without any feedback now. That is not OK, IMO.

iloveeclipse commented 7 months ago

Is there anything missing you need to get motivated to look into it?

Can't say for everyone, but for me: lack of free time & interest in particular compiler area.

Is my reproducer GitHub project somehow inadequate?

If it would be a pure Eclipse project with apt enabled compilation, so it could be easily debugged via starting Eclipse from Eclipse, without using maven / remote debugging maven etc it would be easier for sure. I could at least check if I could see anything obvious wrong there. But maven is the last thing I would like to debug in my free time.

@kriegaex : it is open source project, maintained by a very small team, so contributions from the community (and you are the part of it) is the way to move forward.

srikanth-sankaran commented 7 months ago

And if a plain SDK reproducer is attached and if something more than obvious is seen to be involved per @iloveeclipse's assessment, I offer to investigate,

kriegaex commented 7 months ago

Did the two of you really click on the links I provided? The Maven build is just to download dependencies and reproduce the problem in AspectJ, because it was noticed there first.

a shell script reproducing the problem with ECJ is here.

It does not get more bare-bones than that. Your wish has been fulfilled from the start. Quoting the full batch script:

#!/usr/bin/env bash

export MVN_REPO=../../../.m2/repository
export CP="$MVN_REPO/jakarta/persistence/jakarta.persistence-api/3.1.0/jakarta.persistence-api-3.1.0.jar;$MVN_REPO/com/querydsl/querydsl-apt/5.0.0/querydsl-apt-5.0.0-jakarta.jar;$MVN_REPO/com/querydsl/querydsl-codegen/5.0.0/querydsl-codegen-5.0.0.jar;$MVN_REPO/com/querydsl/codegen-utils/5.0.0/codegen-utils-5.0.0.jar;$MVN_REPO/org/eclipse/jdt/ecj/3.26.0/ecj-3.26.0.jar;$MVN_REPO/javax/inject/javax.inject/1/javax.inject-1.jar;$MVN_REPO/io/github/classgraph/classgraph/4.8.108/classgraph-4.8.108.jar;$MVN_REPO/com/querydsl/querydsl-jpa/5.0.0/querydsl-jpa-5.0.0-jakarta.jar;$MVN_REPO/com/querydsl/querydsl-core/5.0.0/querydsl-core-5.0.0.jar;$MVN_REPO/com/mysema/commons/mysema-commons-lang/0.2.4/mysema-commons-lang-0.2.4.jar;$MVN_REPO/org/aspectj/aspectjrt/1.9.9.1/aspectjrt-1.9.9.1.jar"

export SRC=baz/src/main/java

# Workaround: add '-proc:none' option to ECJ build
java -jar $MVN_REPO/org/eclipse/jdt/ecj/3.36.0/ecj-3.36.0.jar -nowarn -17 -cp "$CP" -d baz/target/classes $SRC/com/foobar/showme/baz/AbstractDurable.java $SRC/com/foobar/showme/baz/Durable.java $SRC/com/foobar/showme/baz/JPADurable.java $SRC/com/foobar/showme/baz/TimestampedJPADurable.java

No Maven at all! I am directly calling ECJ from the console. But I am sure you would have complained, if I had told you to download the necessary dependencies manually, hence the Maven build as a simple way to do that on your behalf.

Whether you want to debug from the console or from Eclipse, is up to you. I wanted to reproduce the problem outside of an IDE, because it is the right thing to do. We all need to be sure that the problem is not caused by IDE settings or mapping them to compiler settings, but by the compiler - or more exactly, its annotation processor - itself. Besides, I do not even use Eclipse IDE, but IntelliJ. But the IDE is irrelevant, we are not talking about an IDE problem.

srikanth-sankaran commented 7 months ago

Did the two of you really click on the links I provided? The Maven build is just to download dependencies and reproduce the problem in AspectJ, because it was noticed here first.

I did not. I don't have maven installed. Strange it may sound to you, I don't know what maven is. I have heard of the name :) I don't know a single thing about it. Honest, I swear. Cross my heart and hope to die :)

a shell script reproducing the problem with ECJ is here.

It does not get more bare-bones than that.

Yes, it can. Very much. We are - correction - I am asking you to invest the time to reduce it to a simple test case by pruning all irrelevant data. This requires effort from your side of course. Give us a minimal Java program, annotation processor code and command line to reproduce.

I am not being confrontational. Simply saying what would allow me to get started on this. I am not speaking for the project. Others may be ready to work on it as it or not.

kriegaex commented 7 months ago

Look, @srikanth-sankaran. I have presented you with more than most people do: a full real-world reproducer. If that is not good enough, I cannot help you. The same applies, if you do not know what Maven is. In that case, simply download all those dependencies by yourself from Maven Central or elsewhere. Sorry for using build tools, because I do not want to download dependencies manually, but this is what I do.

The test case is simple. Admittedly, it could be simpler, but it is simple enough for you to debug and find the root cause, I am certain. Once you know the root cause, probably you will be able to create a minimal test case for yourself.

Sorry, I do not get that sense of entitlement. This is a reproducible bug. What else do you want?

srikanth-sankaran commented 7 months ago

Look, @srikanth-sankaran. I have presented you with more than most people do: a full real-world reproducer. If that is not good enough, I cannot help you. The same applies, if you do not know what Maven is. In that case, simply download all those dependencies by yourself from Maven Central or elsewhere. Sorry for using build tools, because I do not want to download dependencies manually, but this is what I do.

The test case is simple. Admittedly, it could be simpler, but it is simple enough for you to debug and find the root cause, I am certain. Once you know the root cause, probably you will be able to create a minimal test case for yourself.

Sorry, I do not get that sense of entitlement. This is a reproducible bug. What else do you want?

Now, this is confrontational :) Accusing me of a sense of entitlement. I did not accuse of you of entitlement for throwing a set up that is more complex than it needs to be and expecting a fix :)

There are several hundreds of reproducible bugs that I can start today on - What is the probability I am going to start with one that requires upfront investment vs some other defect where the submitter invested time to make it minimal ?

A "full" real world reproducer is not the preferred input. A reduced minimal test case is.

I don't want meta-discussions. My offer to investigate if a standalone reproducer is provided stays. Otherwise I have nothing more to say. Thanks and have a good day.

kriegaex commented 7 months ago

I am not your servant, but your customer. If it is beneath you to investigate a bug with a reproducer and four tiny classes, don't. It tells more about you than about me. And yes, I am being confrontational, because I am not begging for anything here. I do think that a user has a duty to make a problem reproducible. Then, the maintainer has a duty to investigate. It is not the user's duty to cater to all needs and whims of the maintainer, just because the maintainer thinks it is beneath him, if it is not minimal enough.

srikanth-sankaran commented 7 months ago

I did not say it is beneath me - it is not. I personally don't have the time. That is all. Sorry. I wish you the best.

kriegaex commented 7 months ago

Not true. You have time to write here. Within the same time, you could have set up the reproducer and started debugging. I do not buy that excuse.

srikanth-sankaran commented 7 months ago

I am trying to steer the discussions away from personal accusations, but to no avail. I am unsubscribing from this thread. Thanks.

mpalat commented 7 months ago

I am not your servant, but your customer. If it is beneath you to investigate a bug with a reproducer and four tiny classes, don't. It tells more about you than about me. And yes, I am being confrontational, because I am not begging for anything here. I do think that a user has a duty to make a problem reproducible. Then, the maintainer has a duty to investigate. It is not the user's duty to cater to all needs and whims of the maintainer, just because the maintainer thinks it is beneath him, if it is not minimal enough.

@kriegaex - Please tone down. Please understand that everyone has a limited time, and all of us are trying our best to accommodate whatever we can. Please be more professional (and not personal) in your comments. Thank you!

kriegaex commented 7 months ago

@mpalat, what was professional about statements like "lack ... of interest" and "I don't know what Maven is"? I see a missing sense of responsibility, but instead a high sense of entitlement to be pampered by users, blackmailing them into refining an already simple reproducer even more, because the correspondants in question seem to be too lazy or simply unwilling to invest even a minimal, reasonable effort to triage a problem. And what was professional about letting a bug rot for 15 months without any feedback? I had to ask several times, only to get a excuses like people disliking the reproducer, even though using it, the problem is reproducible within a minute.

Why is is acceptable for those people to make their problems my problems and urge me to convert my two choices of Maven and simple shell script to build and run something into something even simpler?

Sorry, but if somebody tries to manipulate me into feeling like my reproducer is inadequate, even though it clearly is adequate (as in able to reproduce a problem with reasonable effort), I take this personally. And if people get personal with me, I play tit for tat.

srikanth-sankaran commented 7 months ago

I did unsubscribe, but keep getting these obnoxious notifications.

I'll speak about just the part that concerns me - There is nothing unprofessional about the assertion "I don't know what maven is" - it is the plain truth. It is not a requirement to know Maven to work on the compiler.

Not knowing what Maven is I have fixed over a 1000 problems in JDT compiler including writing HUGE parts of Java 8 features. I am doing splendidly. Thank you.

Now I'll get back to work. I will not add one more pixel to this thread.

kriegaex commented 7 months ago

I did unsubscribe, but keep getting these obnoxious notifications.

I am sorry for that (no sarcasm, I mean it). I did not expect you to reply anymore.

There is nothing unprofessional about the assertion "I don't know what maven is" - it is the plain truth. It is not a requirement to know Maven to work on the compiler.

I believe that it is the truth. But hey, Maven has been around for a while, and you could just read a quick tutorial. We are developers to learn. You literally just unzip the Maven archive, put it on the path and run the command I wrote into the question. Basic Maven skills would give you access to many reproducers and enable you to do better work faster. Just because you refuse to do that, I am forced to either create a zip file with the dependencies you need to run the reproducer. Sorry, that is a bit too 1980s for me, despite being of advanced age (52) already. You are working on JDT Core, which itself is a Maven project. How can you work with the project without being able to run a build? Instead, you expect me, a mere user of an annotation processor (AP) in one of the reproducer's libraries, to somehow learn how to analyse same AP and then write a new, minimal one for you, so you do not have do call mvn verify. Is that not asking a bit too much?

Not knowing what Maven is I have fixed over a 1000 problems in JDT compiler including writing HUGE parts of Java 8 features. I am doing splendidly.

I believe you, no doubt. But still, you behave like an anti-developer: You refuse to learn even basic skills used to build the very project you are working on.

Maybe I start working on my attitude, if you do.

mpalat commented 7 months ago

@kriegaex Agree that bug have been sitting around for a while but that's because of the limited resources we have and the huge expectations we are subjected to - we here implies the jdt.core committers. And there are definitely more issues that keep coming in - more than we could handle. am not justifying the delay but mentioning the state.

That said, let's see what we can do in this particular bug - I've assigned this 4.32 milestone tentatively. @jarthana would you be able to take a look at this sometime during the 4.32 M1-M3 timeframe as @jarthana is probably available who is (was?) familiar to the annotations part? If yes, please assign to yourself. Else let's see whether someone from the community can take it up. Thanks!

kamilkrzywanski commented 3 months ago
@MappedSuperclass
public abstract class TimestampedJPADurable<I extends Serializable & Comparable<I>, D extends TimestampedJPADurable<I, D>> 
{}

This code is enough to get a StackOverflowError. From my research, it gets stuck at org.eclipse.jdt.internal.compiler.apt.model.TypeVariableImpl.getUpperBound(), and it's the source of the StackOverflowError because the upper bound is always <I extends java.io.Serializable & Comparable>.

getUpperBound() should produce IntersectionType, which is not fully implemented at this factory?

jarthana commented 1 month ago

Thanks for the contribution. Reviewed and approved for RC1. Closing this now.