Bug 547181 - [9][impl] Reconsider representation and lookup of packagesI20190707-1800I20190707-0110I20190706-1800

(SplitPackageBinding)

- replace remaining PlainPackageBinding constructor invocations
- putting packages into ModuleBinding.declaredPackages in
ModuleBinding.addPackage is then not needed, after removing it,
remaining
functionality can be inlined
- removed some unnessary code in MB.getVisiblePackage from the time when
declaredPackages could contain SplitPackageBindings
- always put into cache in MB.getVisiblePackage and remove cache
handling from getTopLevelPackage

Change-Id: Ibd7bddebff4e0315f0f08a5486acd714bc3c46f8

3 files changed, 26 insertions, 66 deletions

diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java
index a8aafd1897..30592db960 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java
@@ -1097,8 +1097,15 @@ public PlainPackageBinding createPlainPackage(char[][] compoundName) {
 	} else {
 		packageBinding = getPackage0(compoundName[0]);
 		if (packageBinding == null || packageBinding == TheNotFoundPackage) {
-			packageBinding = new PlainPackageBinding(compoundName[0], this, this.module);
-			packageBinding = this.module.addPackage(packageBinding, true);
+			packageBinding = this.module.createDeclaredToplevelPackage(compoundName[0]);
+			if (this.useModuleSystem) {
+				char[][] declaringModuleNames = null;
+				if (this.module.isUnnamed()) {
+					IModuleAwareNameEnvironment moduleEnv = (IModuleAwareNameEnvironment) this.nameEnvironment;
+					declaringModuleNames = moduleEnv.getUniqueModulesDeclaringPackage(new char[][] {packageBinding.readableName()}, ModuleBinding.ANY);
+				}
+				packageBinding = this.module.combineWithPackagesFromOtherRelevantModules(packageBinding, packageBinding.compoundName, declaringModuleNames);
+			}
 			this.knownPackages.put(compoundName[0], packageBinding); // update in case of split package
 		}
 	}
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java
index e63a5e22ef..741b9ef056 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ModuleBinding.java
@@ -549,18 +549,7 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 	 * </p>
 	 */
 	public PackageBinding getTopLevelPackage(char[] name) {
-		// check cache:
-		PackageBinding binding = this.environment.getPackage0(name);
-		if (binding != null)
-			return binding == LookupEnvironment.TheNotFoundPackage ? null : binding;
-		binding = getVisiblePackage(null, name);
-		// remember:
-		if (binding != null) {
-			this.environment.knownPackages.put(name, binding);
-		} else {
-			this.environment.knownPackages.put(name, LookupEnvironment.TheNotFoundPackage);
-		}
-		return binding;
+		return getVisiblePackage(null, name);
 	}
 
 	PlainPackageBinding getDeclaredPackage(char[] flatName) {
@@ -579,7 +568,7 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 			if (pkg == LookupEnvironment.TheNotFoundPackage)
 				return null;
 			else
-				return addPackage(pkg, false);
+				return pkg;
 		}
 
 		// check cached plain PackageBinding in declaredPackages (which may need combining with siblings):
@@ -604,7 +593,7 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 						}
 						if (binding == null) {
 							// declared here, not yet known, so create it now:
-							binding = new PlainPackageBinding(subPkgCompoundName, parent, this.environment, this);
+							binding = this.createDeclaredPackage(subPkgCompoundName, parent);
 						}
 					} else {
 						// visible but foreign (when current is unnamed or auto):
@@ -612,14 +601,7 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 							ModuleBinding declaringModule = this.environment.root.getModule(declaringModuleName);
 							if (declaringModule != null) {
 								PlainPackageBinding declaredPackage = declaringModule.getDeclaredPackage(fullFlatName);
-								if (declaredPackage != null) {
-									// don't add foreign package to 'parent' (below), but to its own parent:
-									if (declaredPackage.parent != null)
-										declaredPackage.parent.addPackage(declaredPackage, declaringModule);
-									parent = null;
-									//
-									binding = SplitPackageBinding.combine(declaredPackage, binding, this);
-								}
+								binding = SplitPackageBinding.combine(declaredPackage, binding, this);
 							}
 						}
 					}
@@ -627,32 +609,32 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 			}
 		} else {
 			if (this.environment.nameEnvironment.isPackage(parentName, name))
-				binding = new PlainPackageBinding(subPkgCompoundName, parent, this.environment, this);
+				binding = this.createDeclaredPackage(subPkgCompoundName, parent);
 		}
-
-		// enrich with split-siblings from visible modules:
-		if (parent != null && binding != null)
-			parent.addPackage(binding, this); // preliminarily add to avoid creating duplicates, will be updated below
+		
 		binding = combineWithPackagesFromOtherRelevantModules(binding, subPkgCompoundName, declaringModuleNames);
+
+		assert binding == null || binding instanceof PlainPackageBinding || binding.enclosingModule == this;
+		
 		if (binding == null || !binding.isValidBinding()) {
-			if (parent != null
-					&& !(parent instanceof SplitPackageBinding)) // don't store problem into SPB, because from different focus things may look differently
-			{
+			if (parent != null) {
 				if (binding == null) {
 					parent.addNotFoundPackage(name);
 				} else {
 					parent.knownPackages.put(name, binding);
 				}
+			} else {
+				this.environment.knownPackages.put(name, LookupEnvironment.TheNotFoundPackage);
 			}
 			return null;
 		}
 		// remember
 		if (parentName.length == 0) {
-			binding.environment.knownPackages.put(name, binding);
+			this.environment.knownPackages.put(name, binding);
 		} else if (parent != null) {
 			binding = parent.addPackage(binding, this);
 		}
-		return addPackage(binding, false);
+		return binding;
 	}
 
 	/**
@@ -683,36 +665,7 @@ public class ModuleBinding extends Binding implements IUpdatableModule {
 		return parent;
 	}
 
-	/**
-	 * Check if the given package is declared in this module,
-	 * and if so, remember this fact for later.
-	 * The package can be a {@code SplitPackageBinding} in which case
-	 * only one of its incarnations needs to be declared in this module.
-	 * @param packageBinding the package to add
-	 * @param checkForSplit if true then we should try to construct a split package from
-	 * 	same named packages in required modules.
-	 * @return the given package, possibly enriched to a {@link SplitPackageBinding}
-	 */
-	PackageBinding addPackage(PackageBinding packageBinding, boolean checkForSplit) {
-		if (packageBinding.isDeclaredIn(this)) {
-			char[] packageName = packageBinding.readableName();
-			if (checkForSplit && this.environment.useModuleSystem) {
-				char[][] declaringModuleNames = null;
-				if (isUnnamed()) {
-					IModuleAwareNameEnvironment moduleEnv = (IModuleAwareNameEnvironment) this.environment.nameEnvironment;
-					declaringModuleNames = moduleEnv.getUniqueModulesDeclaringPackage(new char[][] {packageName}, ANY);
-				}
-				packageBinding = combineWithPackagesFromOtherRelevantModules(packageBinding, packageBinding.compoundName, declaringModuleNames);
-			}
-			this.declaredPackages.put(packageName, packageBinding.getIncarnation(this));
-			if (packageBinding.parent == null) {
-				this.environment.knownPackages.put(packageName, packageBinding);
-			}
-		}
-		return packageBinding;
-	}
-	
-	private PackageBinding combineWithPackagesFromOtherRelevantModules(PackageBinding currentBinding, char[][] compoundName, char[][] declaringModuleNames) {
+	PackageBinding combineWithPackagesFromOtherRelevantModules(PackageBinding currentBinding, char[][] compoundName, char[][] declaringModuleNames) {
 		for (ModuleBinding moduleBinding : otherRelevantModules(declaringModuleNames)) {
 			PlainPackageBinding nextBinding = moduleBinding.getDeclaredPackage(CharOperation.concatWith(compoundName, '.'));
 			currentBinding = SplitPackageBinding.combine(nextBinding, currentBinding, this);
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/PlainPackageBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/PlainPackageBinding.java
index 97ea1a2bef..894a710bf8 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/PlainPackageBinding.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/PlainPackageBinding.java
@@ -20,12 +20,12 @@ public class PlainPackageBinding extends PackageBinding {
 
 	/** Create a toplevel package. */
 	public PlainPackageBinding(char[] topLevelPackageName, LookupEnvironment environment, ModuleBinding enclosingModule) {
-		super(new char[][] {topLevelPackageName}, null, environment, enclosingModule);
+		this(new char[][] {topLevelPackageName}, null, environment, enclosingModule);
 	}
 
 	/** Create a default package. */
 	public PlainPackageBinding(LookupEnvironment environment) {
-		super(CharOperation.NO_CHAR_CHAR, null, environment, environment.module);
+		this(CharOperation.NO_CHAR_CHAR, null, environment, environment.module);
 	}
 
 	/** Create a normal package. */