Community
Participate
Working Groups
Compilation error with Java 9 modules. I tried to make a test project as little as possible, only 3 java classes, in 3 different projects. You can download it here: https://www.dropbox.com/s/sz628nqp3j0mxig/TestProject_ModuleCompilationError.zip?dl=0 Tested with eclipse-SDK-I20181104-1800.
Created attachment 276522 [details] Fix I tried to analyse this problem (debug and bisect the git repo), and found that rolling back one specific line helps. It solve the compilation of this test project, and all of my other projects. Of course, this fix is probably not good, I only tested with my projects. I just wanted to help locating the problem.
(In reply to Aurélien Mora from comment #1) > Created attachment 276522 [details] > Fix > > I tried to analyse this problem (debug and bisect the git repo), and found > that rolling back one specific line helps. Could you please contribute this as a Gerrit patch? https://wiki.eclipse.org/JDT_UI/How_to_Contribute
@Andrey, I think this is a regression and he just pointed out the change that broke it.
(In reply to Till Brychcy from comment #3) > @Andrey, I think this is a regression and he just pointed out the change > that broke it. Yes, indeed. I'm really not confident pushing that haha.
(In reply to Aurélien Mora from comment #4) > (In reply to Till Brychcy from comment #3) > > @Andrey, I think this is a regression and he just pointed out the change > > that broke it. > > Yes, indeed. I'm really not confident pushing that haha. I'm actually confident that pushing that change would bring back the blocker bug 537934 :) hence: -2 However, since we have the test project & a pointer to a relevant code location, developing a real fix looks feasible.
(In reply to Stephan Herrmann from comment #5) > I'm actually confident that pushing that change would bring back the blocker > bug 537934 :) > > hence: -2 > > However, since we have the test project & a pointer to a relevant code > location, developing a real fix looks feasible. Yes indeed, the faulty commit was about this bug (1dc06c1721023abc927926c72a35d10decfc4a55).
New Gerrit change created: https://git.eclipse.org/r/132375
(In reply to Eclipse Genie from comment #7) > New Gerrit change created: https://git.eclipse.org/r/132375 Wow, fixing this was a bit of a tour de force, so let me document the story in some detail: The compiler was doing way too much lookup via ModuleBinding.combineWithPackagesFromRequired(). See that the related bug 537934 recently resolved a corresponding FIXME in SPB.combineWithSiblings(). But still, by recursing into getVisiblePackage(char[][]) we forget that we should look only for local packages. This was a pain to debug, in particular the toplevel package "org" was sought gazillions of times. The real problem with this over-eager recursion was, that not-found situations detected in some dependency were persisted in the parent package which could be a split package from a totally different perspective (module). More specifically: - org.sheepy.vulkan is resolving "org.sheepy.vulkan.model.resource" - local package org.sheepy.vulkan is being combined with packages from required modules - we decend into getVisiblePackage() for module org.sheepy.common - we find a parent SplitPackageBinding "org.sheepy.vulkan (from org.sheepy.vulkan.demo, org.sheepy.vulkan)" - inside org.sheepy.common we don't find sub-package "model" - we record into the above split package "org.sheepy.vulkan" that "model" does not exist as a subpackage. - this doesn't hurt during this recursion, but ... - when later (while resolving the import for VulkanBuffer) we find the same split package as a parent and query its subpackage "model", we incorrectly get 'null' as the answer. => Thus org.sheepy.vulkan.model.resource.VulkanBuffer could not be resolved, where it should be. => The primary fix was to add flag 'considerRequiredModules' to more methods to never forget in which mode we are working. => More re-computation is avoided by preferring getVisiblePackage(PackageBinding,char[],boolean) (if parent is known), which avoids starting from the toplevel package over and over again. This caused a few regressions in other tests: The implementation wasn't clear in where and why special treatment should be applied to the unnamed module. In particular the fix for bug 522330 conflicted with one test case. We indeed need to distinguish: - Named modules find non-local packages via all required modules - The UnNamedModule finds non-local packages in all named modules Previously, this distinction was made only in ModuleBinding.addPackage(), but getVisiblePackage() needs to apply the same logic. Since ModuleBinding.getVisiblePackage() already has the names of all modules declaring the sought package, those names can be used as a filter. I pulled this logic into combineWithPackagesFromOtherRelevantModules() (renamed from combineWithPackagesFromRequired), to the effect that getVisiblePackage() is now free of special-casing isUnnamed(). In the end, the corresponding logic could be removed from ModuleBinding.addPackage(), thus further consolidating the code. Still more confusion was caused by moduleEnv.getModulesDeclaringPackage() returning the same module name more than once (specifically during tests with main & test sources). => I fixed this by adding moduleEnv.getUniqueModulesDeclaringPackage() which removes those duplicates. (Since many implementations exist, which potentially have the same problem, wrapping the method in a uniquifying method was simplest). In the interim I saw tests reporting "cannot be resolved" where "is not accessible" is expected - no longer happens after the fix was cleaned-up. Still, if we see this difference in the future, it is easy to fix at the place of error detection by s.t. like: char[][] declaringModules = moduleEnv .getModulesDeclaringPackage(parentName, singleName, ModuleBinding.ANY); if (declaringModules != null && declaringModules.length > 0) { return new ProblemPackageBinding(compoundName, NotAccessible); } I could not easily mend a regression in ModuleBuilderTest.test_ModuleSourcePath_implicitdeps2, but then I found: - it is unstable (fails during debugging, succeeds with no breakpoints) - it accidentally finds inaccessible package from non transitive indirect requirement - it depends on unstable JavaCore.MODULE_PATH_CONTAINER_ID => For these reasons I disabled the test for now. => See also bug 518751
Gerrit change https://git.eclipse.org/r/132375 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=c1cad84a41af37d2e52c34ab44436906251813f0
(In reply to Eclipse Genie from comment #9) > Gerrit change https://git.eclipse.org/r/132375 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=c1cad84a41af37d2e52c34ab44436906251813f0 Released for 4.10 M3
Thank you very much!! This commit helps me a lot, and solve everything for this test model. Unfortunately, I still have a few compilation errors in my full workspace. I'm preparing another test project. I will open another bugzilla.
(In reply to Aurélien Mora from comment #11) > Thank you very much!! > > This commit helps me a lot, and solve everything for this test model. > > Unfortunately, I still have a few compilation errors in my full workspace. > I'm preparing another test project. I will open another bugzilla. Thanks for confirming. Did you perhaps re-test also bug 540293?
*** Bug 540293 has been marked as a duplicate of this bug. ***
(In reply to Aurélien Mora from comment #11) > Thank you very much!! > > This commit helps me a lot, and solve everything for this test model. > > Unfortunately, I still have a few compilation errors in my full workspace. > I'm preparing another test project. I will open another bugzilla. Thanks for confirming. Verified for 4.10 M3 with build id I20181120-1800
