Fix EISOP issue #321 #325
Fix EISOP issue #321 #325
Conversation
framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileElementTypes.java
Outdated
Show resolved
Hide resolved
| */ | ||
| void preProcessTopLevelType(String typeName) { | ||
| boolean success = processingClasses.add(typeName); | ||
| assert success; |
There was a problem hiding this comment.
| assert success; | |
| assert success : "type " + typeName + " is already being processed!"; |
There was a problem hiding this comment.
Does the failure string make it clearer what is going wrong? Can you improve on it?
Instead of success, would changed be clearer?
There was a problem hiding this comment.
I'm also wondering whether to throw a BugInCF instead of an assert - if you're worried that this could arise in a client configuration, the assert is likely not enabled and a BugInCF would ensure we raise an error.
There was a problem hiding this comment.
I think added is more straightforward. I'm also using BugInCF now to ensure we always raise the error.
framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileElementTypes.java
Outdated
Show resolved
Hide resolved
framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileElementTypes.java
Outdated
Show resolved
Hide resolved
framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java
Outdated
Show resolved
Hide resolved
framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java
Outdated
Show resolved
Hide resolved
framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java
Outdated
Show resolved
Hide resolved
framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java
Show resolved
Hide resolved
framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java
Outdated
Show resolved
Hide resolved
| @@ -3119,16 +3149,27 @@ private void stubDebug(String warning) { | |||
| private class AjavaAnnotationCollectorVisitor extends DefaultJointVisitor { | |||
| @Override | |||
| public Void visitClass(ClassTree javacTree, Node javaParserNode) { | |||
| TypeDeclaration<?> typeDecl = (TypeDeclaration<?>) javaParserNode; | |||
There was a problem hiding this comment.
The old code guarded this cast by an instanceof. No test fails with a ClassCastException, but do you see any reason why the previous code used an instanceof?
There was a problem hiding this comment.
Yes, from its super implementation, it seems javaParserNode can have type of LocalClassDeclarationStmt or LocalRecordDeclarationStmt. These two types are not TypeDeclaration but wrappers for ClassOrInterfaceDeclaration and RecordDeclaration, which will be unwrapped by the call super.visitClass(...). So, I need to add the instanceof check back.
…notationFileElementTypes.java Co-authored-by: Werner Dietl <[email protected]>
…notationFileElementTypes.java Co-authored-by: Werner Dietl <[email protected]>
…notationFileParser.java Co-authored-by: Werner Dietl <[email protected]>
…notationFileParser.java Co-authored-by: Werner Dietl <[email protected]>
…notationFileParser.java Co-authored-by: Werner Dietl <[email protected]>
This PR fixes #321 by preventing the parsing of multiple stub files for the same JDK class in the same round.