8159334: ModuleDescriptor retains overlapping sets for all and concealed packages
Reviewed-by: alanb, mchung
--- a/jdk/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java Mon Jun 13 11:46:07 2016 +0200
+++ b/jdk/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java Mon Jun 13 14:02:04 2016 +0200
@@ -790,10 +790,10 @@
private final String osName;
private final String osArch;
private final String osVersion;
- private final Set<String> conceals;
private final Set<String> packages;
private final ModuleHashes hashes;
+
private ModuleDescriptor(String name,
boolean automatic,
boolean synthetic,
@@ -806,7 +806,7 @@
String osName,
String osArch,
String osVersion,
- Set<String> conceals,
+ Set<String> packages,
ModuleHashes hashes)
{
@@ -836,10 +836,7 @@
this.osVersion = osVersion;
this.hashes = hashes;
- assert !exports.keySet().stream().anyMatch(conceals::contains)
- : "Module " + name + ": Package sets overlap";
- this.conceals = emptyOrUnmodifiableSet(conceals);
- this.packages = computePackages(this.exports, this.conceals);
+ this.packages = emptyOrUnmodifiableSet(packages);
}
/**
@@ -862,8 +859,9 @@
this.osVersion = md.osVersion;
this.hashes = null; // need to ignore
- this.packages = emptyOrUnmodifiableSet(pkgs);
- this.conceals = computeConcealedPackages(this.exports, this.packages);
+ Set<String> packages = new HashSet<>(md.packages);
+ packages.addAll(pkgs);
+ this.packages = emptyOrUnmodifiableSet(packages);
}
/**
@@ -882,7 +880,6 @@
String osName,
String osArch,
String osVersion,
- Set<String> conceals,
Set<String> packages,
ModuleHashes hashes) {
this.name = name;
@@ -892,7 +889,6 @@
this.exports = Collections.unmodifiableSet(exports);
this.uses = Collections.unmodifiableSet(uses);
this.provides = Collections.unmodifiableMap(provides);
- this.conceals = Collections.unmodifiableSet(conceals);
this.packages = Collections.unmodifiableSet(packages);
this.version = version;
@@ -1055,7 +1051,9 @@
* @return A possibly-empty unmodifiable set of the concealed packages
*/
public Set<String> conceals() {
- return conceals;
+ Set<String> conceals = new HashSet<>(packages);
+ exports.stream().map(Exports::source).forEach(conceals::remove);
+ return emptyOrUnmodifiableSet(conceals);
}
/**
@@ -1605,6 +1603,8 @@
public ModuleDescriptor build() {
assert name != null;
+ Set<String> packages = new HashSet<>(conceals);
+ packages.addAll(exportedPackages());
return new ModuleDescriptor(name,
automatic,
synthetic,
@@ -1617,7 +1617,7 @@
osName,
osArch,
osVersion,
- conceals,
+ packages,
hashes);
}
@@ -1692,7 +1692,7 @@
&& Objects.equals(osName, that.osName)
&& Objects.equals(osArch, that.osArch)
&& Objects.equals(osVersion, that.osVersion)
- && Objects.equals(conceals, that.conceals)
+ && Objects.equals(packages, that.packages)
&& Objects.equals(hashes, that.hashes));
}
@@ -1723,7 +1723,7 @@
hc = hc * 43 + Objects.hashCode(osName);
hc = hc * 43 + Objects.hashCode(osArch);
hc = hc * 43 + Objects.hashCode(osVersion);
- hc = hc * 43 + Objects.hashCode(conceals);
+ hc = hc * 43 + Objects.hashCode(packages);
hc = hc * 43 + Objects.hashCode(hashes);
if (hc == 0)
hc = -1;
@@ -1879,37 +1879,6 @@
return ModuleInfo.read(bb, null);
}
-
- /**
- * Computes the set of packages from exports and concealed packages.
- * It returns the concealed packages set if there is no exported package.
- */
- private static Set<String> computePackages(Set<Exports> exports,
- Set<String> conceals)
- {
- if (exports.isEmpty())
- return conceals;
-
- Set<String> pkgs = new HashSet<>(conceals);
- exports.stream().map(Exports::source).forEach(pkgs::add);
- return emptyOrUnmodifiableSet(pkgs);
- }
-
- /**
- * Computes the set of concealed packages from exports and all packages.
- * It returns the packages set if there are no exported packages.
- */
- private static Set<String> computeConcealedPackages(Set<Exports> exports,
- Set<String> pkgs)
- {
- if (exports.isEmpty())
- return pkgs;
-
- Set<String> conceals = new HashSet<>(pkgs);
- exports.stream().map(Exports::source).forEach(conceals::remove);
- return emptyOrUnmodifiableSet(conceals);
- }
-
private static <K,V> Map<K,V> emptyOrUnmodifiableMap(Map<K,V> map) {
if (map.isEmpty()) {
return Collections.emptyMap();
@@ -1975,14 +1944,14 @@
boolean automatic,
boolean synthetic,
Set<Requires> requires,
- Set<String> uses, Set<Exports> exports,
+ Set<String> uses,
+ Set<Exports> exports,
Map<String, Provides> provides,
Version version,
String mainClass,
String osName,
String osArch,
String osVersion,
- Set<String> conceals,
Set<String> packages,
ModuleHashes hashes) {
return new ModuleDescriptor(name,
@@ -1997,7 +1966,6 @@
osName,
osArch,
osVersion,
- conceals,
packages,
hashes);
}
--- a/jdk/src/java.base/share/classes/jdk/internal/misc/JavaLangModuleAccess.java Mon Jun 13 11:46:07 2016 +0200
+++ b/jdk/src/java.base/share/classes/jdk/internal/misc/JavaLangModuleAccess.java Mon Jun 13 14:02:04 2016 +0200
@@ -99,7 +99,6 @@
String osName,
String osArch,
String osVersion,
- Set<String> conceals,
Set<String> packages,
ModuleHashes hashes);
--- a/jdk/src/java.base/share/classes/jdk/internal/module/Builder.java Mon Jun 13 11:46:07 2016 +0200
+++ b/jdk/src/java.base/share/classes/jdk/internal/module/Builder.java Mon Jun 13 14:02:04 2016 +0200
@@ -66,8 +66,7 @@
final Set<Requires> requires;
final Set<Exports> exports;
final Map<String, Provides> provides;
- final Set<String> conceals;
- final int numPackages;
+ Set<String> packages;
Set<String> uses;
Version version;
String mainClass;
@@ -78,14 +77,12 @@
Map<String, String> hashes;
Builder(String name, int reqs, int exports,
- int provides, int conceals, int packages) {
+ int provides, int packages) {
this.name = name;
this.requires = reqs > 0 ? new HashSet<>(reqs) : Collections.emptySet();
this.exports = exports > 0 ? new HashSet<>(exports) : Collections.emptySet();
this.provides = provides > 0 ? new HashMap<>(provides) : Collections.emptyMap();
- this.conceals = conceals > 0 ? new HashSet<>(conceals) : Collections.emptySet();
this.uses = Collections.emptySet();
- this.numPackages = packages;
}
/**
@@ -169,18 +166,10 @@
}
/**
- * Adds a set of (possible empty) concealed packages.
+ * Adds a set of (possible empty) packages.
*/
- public Builder conceals(Set<String> packages) {
- conceals.addAll(packages);
- return this;
- }
-
- /**
- * Adds a concealed package.
- */
- public Builder conceals(String pn) {
- conceals.add(pn);
+ public Builder packages(Set<String> packages) {
+ this.packages = packages;
return this;
}
@@ -273,22 +262,6 @@
}
/**
- * Returns the set of packages that is the union of the exported and
- * concealed packages.
- */
- private Set<String> computePackages(Set<Exports> exports, Set<String> conceals) {
- if (exports.isEmpty())
- return conceals;
-
- Set<String> pkgs = new HashSet<>(numPackages);
- pkgs.addAll(conceals);
- for (Exports e : exports) {
- pkgs.add(e.source());
- }
- return pkgs;
- }
-
- /**
* Builds a {@code ModuleDescriptor} from the components.
*/
public ModuleDescriptor build() {
@@ -309,8 +282,7 @@
osName,
osArch,
osVersion,
- conceals,
- computePackages(exports, conceals),
+ packages,
moduleHashes);
}
}
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModuleDescriptorPlugin.java Mon Jun 13 11:46:07 2016 +0200
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModuleDescriptorPlugin.java Mon Jun 13 14:02:04 2016 +0200
@@ -448,29 +448,27 @@
}
void newBuilder(String name, int reqs, int exports, int provides,
- int conceals, int packages) {
+ int packages) {
mv.visitTypeInsn(NEW, MODULE_DESCRIPTOR_BUILDER);
mv.visitInsn(DUP);
mv.visitLdcInsn(name);
pushInt(initialCapacity(reqs));
pushInt(initialCapacity(exports));
pushInt(initialCapacity(provides));
- pushInt(initialCapacity(conceals));
pushInt(initialCapacity(packages));
mv.visitMethodInsn(INVOKESPECIAL, MODULE_DESCRIPTOR_BUILDER,
- "<init>", "(Ljava/lang/String;IIIII)V", false);
+ "<init>", "(Ljava/lang/String;IIII)V", false);
mv.visitVarInsn(ASTORE, BUILDER_VAR);
mv.visitVarInsn(ALOAD, BUILDER_VAR);
}
/*
* Returns the set of concealed packages from ModuleDescriptor, if present
- * or compute it if the module oes not have ConcealedPackages attribute
+ * or compute it if the module does not have ConcealedPackages attribute
*/
Set<String> conceals() {
Set<String> conceals = md.conceals();
- if (md.conceals().isEmpty() &&
- (md.exports().size() + md.conceals().size()) != packages.size()) {
+ if (conceals.isEmpty() && md.exports().size() != packages.size()) {
Set<String> exports = md.exports().stream()
.map(Exports::source)
.collect(Collectors.toSet());
@@ -492,8 +490,7 @@
newBuilder(md.name(), md.requires().size(),
md.exports().size(),
md.provides().size(),
- conceals().size(),
- conceals().size() + md.exports().size());
+ packages.size());
// requires
for (ModuleDescriptor.Requires req : md.requires()) {
@@ -528,10 +525,8 @@
provides(p.service(), p.providers());
}
- // concealed packages
- for (String pn : conceals()) {
- conceals(pn);
- }
+ // all packages
+ packages(packages);
// version
md.version().ifPresent(this::version);
@@ -675,11 +670,13 @@
/*
* Invoke Builder.conceals(String pn)
*/
- void conceals(String pn) {
+ void packages(Set<String> packages) {
mv.visitVarInsn(ALOAD, BUILDER_VAR);
- mv.visitLdcInsn(pn);
+ int varIndex = new StringSetBuilder(packages).build();
+ assert varIndex == STRING_SET_VAR;
+ mv.visitVarInsn(ALOAD, varIndex);
mv.visitMethodInsn(INVOKEVIRTUAL, MODULE_DESCRIPTOR_BUILDER,
- "conceals", STRING_SIG, false);
+ "packages", SET_SIG, false);
mv.visitInsn(POP);
}
@@ -761,7 +758,7 @@
if (localVarIndex == 0) {
// if non-empty and more than one set reference this builder,
// emit to a unique local
- index = refCount == 1 ? STRING_SET_VAR
+ index = refCount <= 1 ? STRING_SET_VAR
: nextLocalVar++;
if (index < MAX_LOCAL_VARS) {
localVarIndex = index;