[java] Fix #5293: Parse number of type parameters eagerly#5309
[java] Fix #5293: Parse number of type parameters eagerly#5309
Conversation
- Improve logging for Parselock Refs pmd#5293
When creating a LazyClassSignature or LazyMethodType, make sure to parse the number of type parameters eagerly, so that AstDisambiguationPass can get this number without triggering additional parsing.
|
|
||
| @Override | ||
| public boolean isGeneric() { | ||
| public int getTypeParameterCount() { |
There was a problem hiding this comment.
We implement getTypeParameterCount() explicitly for ClassStub (the default impl would call signature.getTypeParameters().size() which would trigger the full parsing of the signature).
Note: the default impl of isGeneric() is good enough and uses just the type parameter count. So, no need to override it here.
| this.names = new Names(internalName); | ||
|
|
||
| this.parseLock = new ParseLock() { | ||
| // note to devs: to debug the parsing logic you might have |
There was a problem hiding this comment.
The toString() method in ClassStub already as a comment about avoiding full parsing logic in debug mode. So, this comment is not necessary anymore here.
| protected abstract boolean postCondition(); | ||
|
|
||
| protected abstract boolean isGeneric(); | ||
| protected abstract int getTypeParameterCount(); |
There was a problem hiding this comment.
Each GenericSigBase impl must provide a implementation for getTypeParameterCount()
| super(ctx); | ||
| super(ctx, "LazyClassSignature:" + ctx.getInternalName() + "[" + signature + "]"); | ||
| this.signature = signature; | ||
| this.typeParameterCount = GenericTypeParameterCounter.determineTypeParameterCount(this.signature); |
There was a problem hiding this comment.
Here we parse partially the signature already to determine the type parameter count. That way, we know the number of types, without the need to know the exact types of the type parameters.
| return signature != null && TypeParamsParser.hasTypeParams(signature); | ||
| protected int getTypeParameterCount() { | ||
| // note: no ensureParsed() needed, the type parameters are counted eagerly | ||
| return typeParameterCount; |
There was a problem hiding this comment.
Note: you can easily recreate the bug by adding ensureParsed() here...
Generated by 🚫 Danger |
|
Amazing effort! |
| @Override | ||
| public boolean isGeneric() { | ||
| parseLock.ensureParsed(); | ||
| return signature.isGeneric(); | ||
| } |
There was a problem hiding this comment.
btw. I have no idea, why adding this method would reintroduce the deadlock. At least, the test seems to detect this, in case we accidentally add the deadlock again...
Describe the PR
When creating a LazyClassSignature or LazyMethodType, make sure
to parse the number of type parameters eagerly, so that AstDisambiguationPass
can get this number without triggering additional parsing.
Related issues
Ready?
./mvnw clean verifypasses (checked automatically by github actions)