Community
Participate
Working Groups
In N20100908-2000, there is a compile error reported against PDE: 1. ERROR in /src/org/eclipse/pde/internal/ui/util/OSGiConsole.java (at line 36) session = new ConsoleSession() { The type new ConsoleSession(){} must implement the inherited abstract method ServiceFactory.ungetService(Bundle, ServiceRegistration, Object) However, looking at the code, the ungetService method is actually provided by ConsoleSession, using generics: /** * @noreference This method is not intended to be referenced by clients. * @since 3.7 */ public final void ungetService(Bundle bundle, ServiceRegistration<ConsoleSession> registration, ConsoleSession service) { // do nothing } ConsoleSession - the base class in org.eclipse.osgi, is compiled with 1.5 options, and the leaf class in org.eclipse.pde.ui, is compiled with 1.4 options. Adding the following method to the class in PDE makes the error go away, but I don't think the method should be needed: public void ungetService(Bundle bundle, ServiceRegistration registration, Object service) {}
This is another example of mixed 1.4 and 1.5. OSGi is compiled in 1.5, but it is used as a raw type in pde.ui. If pde.ui is compiled in 1.5 mode, then everything compiles fine without any change.
Created attachment 178519 [details] Stripped-down example projects What's interesting is that the anonymous class compiles fine when the superclass is not abstract.
Note that unlike the previous problems, this is not only broken in the reconciler. I guess the problem is that the anonymous ConsoleSession is missing the synthetic bridge method. Adding this to the new ConsoleSession() {..} solves the compile error: public void ungetService(Bundle bundle, ServiceRegistration registration, Object service) { ungetService(bundle, registration, (ConsoleSession) service); }
Created attachment 178526 [details] Regression test This disabled regression test should help to track this issue down. Everything is fine if the client code is compiled in 1.5 mode or if the class is not abstract.
I committed the regression test.
I am likely going to backout the changes to ConsoleSession that caused this issue in the build for now.
(In reply to comment #6) > I am likely going to backout the changes to ConsoleSession that caused this > issue in the build for now. Don't revert to raw. Just change the generic types to <ConsoleSession, Object> to get the desired erasure (and add some necessary casts since the tracked type is now Object).
Created attachment 178543 [details] ConsoleSession patch (In reply to comment #7) > (In reply to comment #6) > > I am likely going to backout the changes to ConsoleSession that caused this > > issue in the build for now. > > Don't revert to raw. Just change the generic types to <ConsoleSession, Object> > to get the desired erasure (and add some necessary casts since the tracked type > is now Object). We are really just talking about ConsoleSession which implements ServiceFactory<ConsoleSession>. I think that needs to change to ServiceFactory<Object> to work around this bug. This should not affect the code that does the actual tracking of ConsoleSession services.
I released the ConsoleSession patch to allow the nightly build to succeed tonight.
This case shows the unpreparedness of JDT/Core to deal with this sort of mixing of projects. Or for a 1.4 having supertypes from 1.5 project. Basically, it would appear it is a folly to drop any generic information if present at all. Also wherever compiler code is partitioned into two parallel universes for pre-generics & post-generics we are going to have trouble. For instance the current problem comes at least partly from the compiler using 1.4 method verification code. Under investigation.
(In reply to comment #2) > What's interesting is that the anonymous class compiles fine when the > superclass is not abstract. That is because the "method verifier", the piece of the compiler that makes sure that a type fulfills its contractual obligations can assume that the super class must have implemented a concrete version of the method which the current class will automatically inherit. Failing which it can assume that the error would have been reported against the super class. (In reply to comment #3) > Note that unlike the previous problems, this is not only broken in the > reconciler. I have a patch that is under test that fixes the problem in the compiler proper, though we are back to the same issue of the schizophrenic name lookup environment as far as the reconciler is concerned. If the patch holds up to scrutiny and test, I would like to split this defect and create a fresh one for the reconciler and close the current one.
Created attachment 179022 [details] Preliminary patch under test
Comment on attachment 179022 [details] Preliminary patch under test Fails some tests, back to investigation
Created attachment 179624 [details] Patch under test This patch fixes the problem in both the reconciler and the compiler. This is under test. Since some very basic things are totally rewired in this patch (e.g, we now use MethodVerifier15 to verify the contracts implemented by even <= 1.4 types), this patch needs to be put through torture test before release and will likely need more massaging before it is fully cooked. Stay tuned.
Created attachment 179799 [details] Revised patch under test This patch includes adjustments to the expected output of a bunch of JDT/Core tests that are seeing different diagnostics due to the admission of type variables in scenarios where they would previously be dropped. For the most part the new diagnostics are better as they avoid secondary errors.
Created attachment 179821 [details] Revised patch - passes all JDT/Core tests Will test some more before code review and release.
(In reply to comment #16) > Created an attachment (id=179821) [details] > Revised patch - passes all JDT/Core tests > > Will test some more before code review and release. Deepak (Thanks!) confirmed that all JDT/UI tests pass too.
Satyam, please review - TIA.
Olivier, please also review. Since this is a major change in some ways, I'll release after I return my vacation (18th Oct) so I can be available to quickly work on any ripples this results in.
As far as I can say, the patch looks good.
The changes look good to me.
Released in HEAD for 3.7M3
Verified for 3.7M3 using build I20101025-0901.
Revert in bug 328775 comment 4. Will be released again as soon as M3 is declared.
(Re)Released in HEAD for 3.7 M4
I have also released the fixes for bug 328689 bug 328775 and bug 328827. None of these 3 bugs were directly brought about by the fix for the current bug though they were exposed by the fix to bug 324850 - there could be more problems lurking - we would like to request some early testing via the upcoming I builds to catch these problem well in time to be addressed for M4
verified for 3.7M4 using build I20101205-2000.
