Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8236842: Surprising 'multiple elements' behaviour from getTypeElement when cross-compiling with --release #200

Closed
wants to merge 5 commits into from
Closed
@@ -147,7 +147,7 @@
* <p> Otherwise, resolution succeeds, and the result of resolution is the
* readability graph.
*
* <h3> Root modules </h3>
* <h3><a id="root-modules"></a> Root modules </h3>
*
* <p> The set of root modules at compile-time is usually the set of modules
* being compiled. At run-time, the set of root modules is usually the
@@ -51,11 +51,30 @@
public interface Elements {

/**
* Returns a package given its fully qualified name if the package is unique in the environment.
* If running with modules, all modules in the modules graph are searched for matching packages.
*
* @param name fully qualified package name, or an empty string for an unnamed package
* @return the specified package, or {@code null} if it cannot be uniquely found
* Returns a package given its fully qualified name if the package is uniquely
* determinable in the environment.
*
* If running with modules, packages of the given name are searched in a
* two-stage process:
* <ul>
* <li>find non-empty packages with the given name returned by
* {@link #getPackageElement(ModuleElement, CharSequence)},
* where the provided ModuleSymbol is any
* <a href="../../../../../java.base/java/lang/module/package-summary.html#root-modules">root module</a>,
* </li>
* <li>if the above yields an empty list, search
* {@link #getAllModuleElements() all modules} for observable
* packages with the given name
* </li>
* </ul>
*
* If this process leads to a list with a single element,
* the single element is returned, otherwise null is returned.
*
* @param name fully qualified package name,
* or an empty string for an unnamed package
* @return the specified package,
* or {@code null} if no package can be uniquely determined.
*/
PackageElement getPackageElement(CharSequence name);

@@ -119,12 +138,29 @@ default PackageElement getPackageElement(ModuleElement module, CharSequence name
}

/**
* Returns a type element given its canonical name if the type element is unique in the environment.
* If running with modules, all modules in the modules graph are searched for matching
* type elements.
*
* @param name the canonical name
* @return the named type element, or {@code null} if it cannot be uniquely found
* Returns a type element given its canonical name if the type element is uniquely
* determinable in the environment.
*
* If running with modules, type elements of the given name are
* searched in a two-stage process:
* <ul>
* <li>find type elements with the given name returned by
* {@link #getTypeElement(ModuleElement, CharSequence)},
* where the provided ModuleSymbol is any
* <a href="../../../../../java.base/java/lang/module/package-summary.html#root-modules">root module</a>,
* </li>
* <li>if the above yields an empty list, search
* {@link #getAllModuleElements() all modules} for observable
* type elements with the given name
* </li>
* </ul>
*
* If this process leads to a list with a single element,
* the single element is returned, otherwise null is returned.
*
* @param name the canonical name
* @return the named type element,
* or {@code null} if no type element can be uniquely determined.
*/
TypeElement getTypeElement(CharSequence name);

@@ -25,6 +25,7 @@

package com.sun.tools.javac.model;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -198,43 +199,48 @@ private ClassSymbol doGetTypeElement(ModuleElement module, CharSequence name) {

return (S) resultCache.computeIfAbsent(Pair.of(methodName, nameStr), p -> {
Set<S> found = new LinkedHashSet<>();
Set<ModuleSymbol> allModules = new HashSet<>(modules.allModules());

for (ModuleSymbol msym : modules.allModules()) {
S sym = nameToSymbol(msym, nameStr, clazz);
allModules.removeAll(modules.getRootModules());

if (sym == null)
continue;
for (Set<ModuleSymbol> modules : Arrays.asList(modules.getRootModules(), allModules)) {
for (ModuleSymbol msym : modules) {
S sym = nameToSymbol(msym, nameStr, clazz);

if (sym == null)
continue;

if (clazz == ClassSymbol.class) {
// Always include classes
found.add(sym);
} else if (clazz == PackageSymbol.class) {
// In module mode, ignore the "spurious" empty packages that "enclose" module-specific packages.
// For example, if a module contains classes or package info in package p.q.r, it will also appear
// to have additional packages p.q and p, even though these packages have no content other
// than the subpackage. We don't want those empty packages showing up in searches for p or p.q.
if (!sym.members().isEmpty() || ((PackageSymbol) sym).package_info != null) {
if (clazz == ClassSymbol.class) {
// Always include classes
found.add(sym);
} else if (clazz == PackageSymbol.class) {
// In module mode, ignore the "spurious" empty packages that "enclose" module-specific packages.
// For example, if a module contains classes or package info in package p.q.r, it will also appear
// to have additional packages p.q and p, even though these packages have no content other
// than the subpackage. We don't want those empty packages showing up in searches for p or p.q.
if (!sym.members().isEmpty() || ((PackageSymbol) sym).package_info != null) {
found.add(sym);
}
}
}
}

if (found.size() == 1) {
return Optional.of(found.iterator().next());
} else if (found.size() > 1) {
//more than one element found, produce a note:
if (alreadyWarnedDuplicates.add(methodName + ":" + nameStr)) {
String moduleNames = found.stream()
.map(s -> s.packge().modle)
.map(m -> m.toString())
.collect(Collectors.joining(", "));
log.note(Notes.MultipleElements(methodName, nameStr, moduleNames));
if (found.size() == 1) {
return Optional.of(found.iterator().next());
} else if (found.size() > 1) {
//more than one element found, produce a note:
if (alreadyWarnedDuplicates.add(methodName + ":" + nameStr)) {
String moduleNames = found.stream()
.map(s -> s.packge().modle)
.map(m -> m.toString())
.collect(Collectors.joining(", "));
log.note(Notes.MultipleElements(methodName, nameStr, moduleNames));
}
return Optional.empty();
} else {
//not found, try another option

This comment has been minimized.

@vicente-romero-oracle

vicente-romero-oracle Sep 16, 2020
Contributor

probably this else {} can be removed right?

This comment has been minimized.

@lahodaj

lahodaj Sep 17, 2020
Author Contributor

It could be removed, but I added it intentionally to make it clear the "else" option has been considered, and is intentional. I can remove it, if you strongly prefer, though.

}
return Optional.empty();
} else {
//not found:
return Optional.empty();
}
return Optional.empty();
}).orElse(null);
}

ProTip! Use n and p to navigate between commits in a pull request.