Bugzilla – Full Text Bug Listing
| Summary: | [1.7] CCE when using diamond in 1.4 | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Markus Keller <markus.kell.r> | ||||||||||
| Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||
| Severity: | major | ||||||||||||
| Priority: | P3 | CC: | amj87.iitr, raksha.vasisht, srikanth_sankaran | ||||||||||
| Version: | 3.7 | ||||||||||||
| Target Milestone: | 3.7.1 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Windows 7 | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
Seems like one example of what I was trying to say in https://bugs.eclipse.org/bugs/show_bug.cgi?id=348493#c9 Srikanth, what would you suggest - protecting the casts with an instanceof check, or putting the source level checks at all diamond usages? I'm still in favour of the latter. (In reply to comment #2) > Srikanth, what would you suggest - protecting the casts with an instanceof > check, or putting the source level checks at all diamond usages? I'm still in > favour of the latter. I would go with instanceof checks in places relevant to the bug at hand. For reasons already discussed I am not in favor of littering the code with source level checks. In other contexts, this error repair strategy works alright and I don't see why it won't in the current instance. (In reply to comment #3) > I would go with instanceof checks in places relevant to the bug at hand. > For reasons already discussed I am not in favor of littering the code with > source level checks. In other contexts, this error repair strategy works > alright and I don't see why it won't in the current instance. Is it just a question of choosing between clean code and the safest possible code. There may be always corner cases where something unexpected will leak out somewhere. And then we need to firefight. Shouldn't we take a more precautionary approach here, since its also harmless? Created attachment 199592 [details] Early patch (In reply to comment #4) > (In reply to comment #3) > > I would go with instanceof checks in places relevant to the bug at hand. > > For reasons already discussed I am not in favor of littering the code with > > source level checks. In other contexts, this error repair strategy works > > alright and I don't see why it won't in the current instance. > > Is it just a question of choosing between clean code and the safest possible > code. There may be always corner cases where something unexpected will leak out > somewhere. And then we need to firefight. Shouldn't we take a more > precautionary approach here, since its also harmless? No, we don't want to be dogmatic about clean code. Let us also add mindless checks in the name of safety - I am fine if there was some analysis to back these checks. Please take a look at this patch. This is untested, but this may offer a solution that is clean as well as safe ? (In reply to comment #5) > No, we don't want to be dogmatic about clean code. Let us also add mindless > checks in the name of safety No, that is not what I meant, of course :) If this patch doesn't pan out, I would suggest you study the existing uses of <> to see where we need to add defensive checks and add it only there. An alternate approach is to flip the isDiamond flag the moment the error has been reported - There may be more to it than just that. Created attachment 199596 [details]
Revised patch
Same patch with comments in the right place.
FWIW, I quickly hacked the ASTParser and our test suites so that I can run all UI tests with a 1.3 rt.jar and generate ASTs with compliance level 1.4. This causes many failures because the rt.jar is not generified, and a few errors for other reasons. The only remaining problems I found were this bug and a test case for bug 300576. I've verified that the patch from comment 7 resolves all CCEs. (In reply to comment #7) > Created attachment 199596 [details] [diff] > Revised patch > > Same patch with comments in the right place. I couldn't really come up with cases where there would be some exception for <1.7 compliance apart from the above. I also ran our 1.7 tests on 1.4 compliance to see if something breaks. All looks fine to me. So the patch should be sufficient. I'll add a test case and release Created attachment 199629 [details]
Alternate patch
This should do the trick too. (Basically, beefing up resilience.)
(In reply to comment #10) > Created attachment 199629 [details] > Alternate patch > > This should do the trick too. (Basically, beefing up resilience.) Let us go with the patch from comment#7 itself as it has seen better testing. Created attachment 199635 [details] fix + tests Added tests to patch in comment 7 Released in BETA_JAVA7 branch. Verified. |
BETA_JAVA7 Try to compile this snippet in a 1.4 project: package p; import java.util.ArrayList; public class C { public static void main(String[] args) { new ArrayList<>(); } } Seems like the compiler doesn't like the diamond: !ENTRY org.eclipse.jdt.ui 4 0 2011-07-13 14:59:08.424 !MESSAGE Error in JDT Core during AST creation !STACK 0 java.lang.ClassCastException: org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding cannot be cast to org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding at org.eclipse.jdt.internal.compiler.ast.AllocationExpression.resolveType(AllocationExpression.java:380) at org.eclipse.jdt.internal.compiler.ast.Expression.resolve(Expression.java:939) at org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration.resolveStatements(AbstractMethodDeclaration.java:463) at org.eclipse.jdt.internal.compiler.ast.MethodDeclaration.resolveStatements(MethodDeclaration.java:252) at org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration.resolve(AbstractMethodDeclaration.java:422) at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.resolve(TypeDeclaration.java:1148) at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.resolve(TypeDeclaration.java:1258) at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.resolve(CompilationUnitDeclaration.java:539) at org.eclipse.jdt.core.dom.CompilationUnitResolver.resolve(CompilationUnitResolver.java:1181) at org.eclipse.jdt.core.dom.CompilationUnitResolver.resolve(CompilationUnitResolver.java:681) at org.eclipse.jdt.core.dom.ASTParser.internalCreateAST(ASTParser.java:1185) at org.eclipse.jdt.core.dom.ASTParser.createAST(ASTParser.java:811) at org.eclipse.jdt.internal.ui.javaeditor.ASTProvider$1.run(ASTProvider.java:548) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.jdt.internal.ui.javaeditor.ASTProvider.createAST(ASTProvider.java:541) at org.eclipse.jdt.internal.ui.javaeditor.ASTProvider.getAST(ASTProvider.java:484) at org.eclipse.jdt.internal.ui.javaeditor.ASTProvider.getAST(ASTProvider.java:474) at org.eclipse.jdt.ui.SharedASTProvider.getAST(SharedASTProvider.java:132) at org.eclipse.jdt.internal.ui.viewsupport.SelectionListenerWithASTManager$PartListenerGroup.calculateASTandInform(SelectionListenerWithASTManager.java:170) at org.eclipse.jdt.internal.ui.viewsupport.SelectionListenerWithASTManager$PartListenerGroup$3.run(SelectionListenerWithASTManager.java:155) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)