8173117: Compilation significantly slower after JDK-8169197
authorjlahoda
Fri, 20 Jan 2017 15:32:07 +0100
changeset 43272 421ae1e38d2d
parent 43271 ce89609dde7c
child 43273 2614e1907a0b
8173117: Compilation significantly slower after JDK-8169197 Summary: Only using recovery search when an error is inevitable. Reviewed-by: jjg, mcimadamore
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/model/JavacElements.java
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Convert.java
langtools/test/tools/javac/modules/ConvenientAccessErrorsTest.java
langtools/test/tools/javac/modules/EdgeCases.java
langtools/test/tools/javac/modules/PackageMultipleModules.java
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java	Fri Jan 20 15:32:03 2017 +0100
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java	Fri Jan 20 15:32:07 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -818,4 +818,12 @@
     public Collection<ModuleSymbol> getAllModules() {
         return modules.values();
     }
+
+    public Iterable<ClassSymbol> getClassesForName(Name candidate) {
+        return classes.getOrDefault(candidate, Collections.emptyMap()).values();
+    }
+
+    public Iterable<PackageSymbol> getPackagesForName(Name candidate) {
+        return packages.getOrDefault(candidate, Collections.emptyMap()).values();
+    }
 }
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java	Fri Jan 20 15:32:03 2017 +0100
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java	Fri Jan 20 15:32:07 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -41,6 +41,7 @@
 import com.sun.tools.javac.comp.Resolve.ReferenceLookupResult.StaticKind;
 import com.sun.tools.javac.jvm.*;
 import com.sun.tools.javac.main.Option;
+import com.sun.tools.javac.resources.CompilerProperties.Fragments;
 import com.sun.tools.javac.tree.*;
 import com.sun.tools.javac.tree.JCTree.*;
 import com.sun.tools.javac.tree.JCTree.JCMemberReference.ReferenceKind;
@@ -61,12 +62,12 @@
 import java.util.Set;
 import java.util.function.BiFunction;
 import java.util.function.BiPredicate;
+import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
 
 import javax.lang.model.element.ElementVisitor;
 
-import com.sun.tools.javac.code.Directive.ExportsDirective;
 import static com.sun.tools.javac.code.Flags.*;
 import static com.sun.tools.javac.code.Flags.BLOCK;
 import static com.sun.tools.javac.code.Flags.STATIC;
@@ -74,9 +75,8 @@
 import static com.sun.tools.javac.code.Kinds.Kind.*;
 import static com.sun.tools.javac.code.TypeTag.*;
 import static com.sun.tools.javac.comp.Resolve.MethodResolutionPhase.*;
-import com.sun.tools.javac.resources.CompilerProperties.Errors;
-import com.sun.tools.javac.resources.CompilerProperties.Fragments;
 import static com.sun.tools.javac.tree.JCTree.Tag.*;
+import static com.sun.tools.javac.util.Iterators.createCompoundIterator;
 
 /** Helper class for name resolution, used mostly by the attribution phase.
  *
@@ -1970,7 +1970,7 @@
      *  @param env       The current environment.
      *  @param name      The fully qualified name of the class to be loaded.
      */
-    Symbol loadClass(Env<AttrContext> env, Name name) {
+    Symbol loadClass(Env<AttrContext> env, Name name, RecoveryLoadClass recoveryLoadClass) {
         try {
             ClassSymbol c = finder.loadClass(env.toplevel.modle, name);
             return isAccessible(env, c) ? c : new AccessError(env, null, c);
@@ -1987,40 +1987,60 @@
         }
     }
 
-    public static interface RecoveryLoadClass {
+    public interface RecoveryLoadClass {
         Symbol loadClass(Env<AttrContext> env, Name name);
     }
 
-    private RecoveryLoadClass recoveryLoadClass = new RecoveryLoadClass() {
-        @Override
-        public Symbol loadClass(Env<AttrContext> env, Name name) {
-            if (allowModules) {
-                Scope importScope = env.toplevel.namedImportScope;
-                Symbol existing = importScope.findFirst(Convert.shortName(name),
-                                                        sym -> sym.kind == TYP && sym.flatName() == name);
-
-                if (existing != null) {
-                    return new InvisibleSymbolError(env, true, existing);
-                }
-
-                return lookupInvisibleSymbol(env, name, syms::getClass, (ms, n) -> {
+    private final RecoveryLoadClass noRecovery = (env, name) -> null;
+
+    private final RecoveryLoadClass doRecoveryLoadClass = new RecoveryLoadClass() {
+        @Override public Symbol loadClass(Env<AttrContext> env, Name name) {
+            List<Name> candidates = Convert.classCandidates(name);
+            return lookupInvisibleSymbol(env, name,
+                                         n -> () -> createCompoundIterator(candidates,
+                                                                           c -> syms.getClassesForName(c)
+                                                                                    .iterator()),
+                                         (ms, n) -> {
+                for (Name candidate : candidates) {
                     try {
-                        return finder.loadClass(ms, n);
+                        return finder.loadClass(ms, candidate);
                     } catch (CompletionFailure cf) {
                         //ignore
-                        return null;
                     }
-                }, sym -> sym.kind == Kind.TYP, false, typeNotFound);
-            }
-            return null;
+                }
+                return null;
+            }, sym -> sym.kind == Kind.TYP, false, typeNotFound);
         }
     };
 
-    public RecoveryLoadClass setRecoveryLoadClass(RecoveryLoadClass recovery) {
-        RecoveryLoadClass prev = recoveryLoadClass;
-        recoveryLoadClass = recovery;
-        return prev;
-    }
+    private final RecoveryLoadClass namedImportScopeRecovery = (env, name) -> {
+        Scope importScope = env.toplevel.namedImportScope;
+        Symbol existing = importScope.findFirst(Convert.shortName(name),
+                                                sym -> sym.kind == TYP && sym.flatName() == name);
+
+        if (existing != null) {
+            return new InvisibleSymbolError(env, true, existing);
+        }
+        return null;
+    };
+
+    private final RecoveryLoadClass starImportScopeRecovery = (env, name) -> {
+        Scope importScope = env.toplevel.starImportScope;
+        Symbol existing = importScope.findFirst(Convert.shortName(name),
+                                                sym -> sym.kind == TYP && sym.flatName() == name);
+
+        if (existing != null) {
+            try {
+                existing = finder.loadClass(existing.packge().modle, name);
+
+                return new InvisibleSymbolError(env, true, existing);
+            } catch (CompletionFailure cf) {
+                //ignore
+            }
+        }
+
+        return null;
+    };
 
     Symbol lookupPackage(Env<AttrContext> env, Name name) {
         PackageSymbol pack = syms.lookupPackage(env.toplevel.modle, name);
@@ -2034,7 +2054,7 @@
                                                           .stream()
                                                           .anyMatch(p -> p.fullname.startsWith(nameAndDot));
 
-                return lookupInvisibleSymbol(env, name, syms::getPackage, syms::enterPackage, sym -> {
+                return lookupInvisibleSymbol(env, name, syms::getPackagesForName, syms::enterPackage, sym -> {
                     sym.complete();
                     return sym.exists();
                 }, prefixOfKnown, pack);
@@ -2059,42 +2079,44 @@
         return TreeInfo.fullName(((JCFieldAccess) qualid).selected) == name;
     }
 
-    private Symbol lookupInvisibleSymbol(Env<AttrContext> env,
-                                         Name name,
-                                         BiFunction<ModuleSymbol, Name, Symbol> get,
-                                         BiFunction<ModuleSymbol, Name, Symbol> load,
-                                         Predicate<Symbol> validate,
-                                         boolean suppressError,
-                                         Symbol defaultResult) {
+    private <S extends Symbol> Symbol lookupInvisibleSymbol(Env<AttrContext> env,
+                                                            Name name,
+                                                            Function<Name, Iterable<S>> get,
+                                                            BiFunction<ModuleSymbol, Name, S> load,
+                                                            Predicate<S> validate,
+                                                            boolean suppressError,
+                                                            Symbol defaultResult) {
         //even if a class/package cannot be found in the current module and among packages in modules
         //it depends on that are exported for any or this module, the class/package may exist internally
         //in some of these modules, or may exist in a module on which this module does not depend.
         //Provide better diagnostic in such cases by looking for the class in any module:
+        Iterable<? extends S> candidates = get.apply(name);
+
+        for (S sym : candidates) {
+            if (validate.test(sym))
+                return new InvisibleSymbolError(env, suppressError, sym);
+        }
+
         Set<ModuleSymbol> recoverableModules = new HashSet<>(syms.getAllModules());
 
         recoverableModules.remove(env.toplevel.modle);
 
         for (ModuleSymbol ms : recoverableModules) {
-            Symbol sym = get.apply(ms, name);
-
             //avoid overly eager completing classes from source-based modules, as those
             //may not be completable with the current compiler settings:
-            if (sym == null && (ms.sourceLocation == null)) {
+            if (ms.sourceLocation == null) {
                 if (ms.classLocation == null) {
                     ms = moduleFinder.findModule(ms);
                 }
 
                 if (ms.kind != ERR) {
-                    sym = load.apply(ms, name);
+                    S sym = load.apply(ms, name);
+
+                    if (sym != null && validate.test(sym)) {
+                        return new InvisibleSymbolError(env, suppressError, sym);
+                    }
                 }
             }
-
-            if (sym == null)
-                continue;
-
-            if (validate.test(sym)) {
-                return new InvisibleSymbolError(env, suppressError, sym);
-            }
         }
 
         return defaultResult;
@@ -2186,10 +2208,10 @@
      *  @param scope     The scope in which to look for the type.
      *  @param name      The type's name.
      */
-    Symbol findGlobalType(Env<AttrContext> env, Scope scope, Name name) {
+    Symbol findGlobalType(Env<AttrContext> env, Scope scope, Name name, RecoveryLoadClass recoveryLoadClass) {
         Symbol bestSoFar = typeNotFound;
         for (Symbol s : scope.getSymbolsByName(name)) {
-            Symbol sym = loadClass(env, s.flatName());
+            Symbol sym = loadClass(env, s.flatName(), recoveryLoadClass);
             if (bestSoFar.kind == TYP && sym.kind == TYP &&
                 bestSoFar != sym)
                 return new AmbiguityError(bestSoFar, sym);
@@ -2260,15 +2282,15 @@
         }
 
         if (!env.tree.hasTag(IMPORT)) {
-            sym = findGlobalType(env, env.toplevel.namedImportScope, name);
+            sym = findGlobalType(env, env.toplevel.namedImportScope, name, namedImportScopeRecovery);
             if (sym.exists()) return sym;
             else bestSoFar = bestOf(bestSoFar, sym);
 
-            sym = findGlobalType(env, env.toplevel.packge.members(), name);
+            sym = findGlobalType(env, env.toplevel.packge.members(), name, noRecovery);
             if (sym.exists()) return sym;
             else bestSoFar = bestOf(bestSoFar, sym);
 
-            sym = findGlobalType(env, env.toplevel.starImportScope, name);
+            sym = findGlobalType(env, env.toplevel.starImportScope, name, starImportScopeRecovery);
             if (sym.exists()) return sym;
             else bestSoFar = bestOf(bestSoFar, sym);
         }
@@ -2315,7 +2337,11 @@
         Name fullname = TypeSymbol.formFullName(name, pck);
         Symbol bestSoFar = typeNotFound;
         if (kind.contains(KindSelector.TYP)) {
-            Symbol sym = loadClass(env, fullname);
+            RecoveryLoadClass recoveryLoadClass =
+                    allowModules && !kind.contains(KindSelector.PCK) &&
+                    !pck.exists() && !env.info.isSpeculative ?
+                        doRecoveryLoadClass : noRecovery;
+            Symbol sym = loadClass(env, fullname, recoveryLoadClass);
             if (sym.exists()) {
                 // don't allow programs to use flatnames
                 if (name == sym.name) return sym;
@@ -4136,11 +4162,21 @@
 
             JCDiagnostic details = inaccessiblePackageReason(env, sym.packge());
 
-            if (pos.getTree() != null && pos.getTree().hasTag(SELECT) && sym.owner.kind == PCK) {
-                pos = ((JCFieldAccess) pos.getTree()).selected.pos();
-
-                return diags.create(dkind, log.currentSource(),
-                        pos, "package.not.visible", sym.packge(), details);
+            if (pos.getTree() != null) {
+                Symbol o = sym;
+                JCTree tree = pos.getTree();
+
+                while (o.kind != PCK && tree.hasTag(SELECT)) {
+                    o = o.owner;
+                    tree = ((JCFieldAccess) tree).selected;
+                }
+
+                if (o.kind == PCK) {
+                    pos = tree.pos();
+
+                    return diags.create(dkind, log.currentSource(),
+                            pos, "package.not.visible", o, details);
+                }
             }
 
             return diags.create(dkind, log.currentSource(),
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/model/JavacElements.java	Fri Jan 20 15:32:03 2017 +0100
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/model/JavacElements.java	Fri Jan 20 15:32:07 2017 +0100
@@ -184,39 +184,34 @@
             return nameToSymbol(syms.noModule, nameStr, clazz);
         }
 
-        RecoveryLoadClass prevRecoveryLoadClass = resolve.setRecoveryLoadClass((env, name) -> null);
-        try {
-            Set<S> found = new LinkedHashSet<>();
+        Set<S> found = new LinkedHashSet<>();
+
+        for (ModuleSymbol msym : modules.allModules()) {
+            S sym = nameToSymbol(msym, nameStr, clazz);
 
-            for (ModuleSymbol msym : modules.allModules()) {
-                S sym = nameToSymbol(msym, nameStr, clazz);
-
-                if (sym != null) {
-                    if (!allowModules || clazz == ClassSymbol.class || !sym.members().isEmpty()) {
-                        //do not add packages without members:
-                        found.add(sym);
-                    }
+            if (sym != null) {
+                if (!allowModules || clazz == ClassSymbol.class || !sym.members().isEmpty()) {
+                    //do not add packages without members:
+                    found.add(sym);
                 }
             }
+        }
 
-            if (found.size() == 1) {
-                return 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 null;
-            } else {
-                //not found, or more than one element found:
-                return null;
+        if (found.size() == 1) {
+            return 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));
             }
-        } finally {
-            resolve.setRecoveryLoadClass(prevRecoveryLoadClass);
+            return null;
+        } else {
+            //not found, or more than one element found:
+            return null;
         }
     }
 
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Convert.java	Fri Jan 20 15:32:03 2017 +0100
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Convert.java	Fri Jan 20 15:32:07 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -335,4 +335,16 @@
         }
         return names;
     }
+
+    public static List<Name> classCandidates(Name name) {
+        List<Name> names = List.nil();
+        String nameStr = name.toString();
+        int index = -1;
+        while ((index = nameStr.indexOf('.', index + 1)) > 0) {
+            String pack = nameStr.substring(0, index + 1);
+            String clz = nameStr.substring(index + 1).replace('.', '$');
+            names = names.prepend(name.table.names.fromString(pack + clz));
+        }
+        return names.reverse();
+    }
 }
--- a/langtools/test/tools/javac/modules/ConvenientAccessErrorsTest.java	Fri Jan 20 15:32:03 2017 +0100
+++ b/langtools/test/tools/javac/modules/ConvenientAccessErrorsTest.java	Fri Jan 20 15:32:07 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,11 +23,12 @@
 
 /*
  * @test
- * @bug 8169197 8172668
+ * @bug 8169197 8172668 8173117
  * @summary Check convenient errors are produced for inaccessible classes.
  * @library /tools/lib
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
+ *          jdk.compiler/com.sun.tools.javac.util
  * @build toolbox.ToolBox toolbox.JarTask toolbox.JavacTask ModuleTestBase
  * @run main ConvenientAccessErrorsTest
  */
@@ -37,6 +38,10 @@
 import java.util.Arrays;
 import java.util.List;
 
+import com.sun.tools.javac.util.Context;
+import com.sun.tools.javac.util.Convert;
+import com.sun.tools.javac.util.Name;
+import com.sun.tools.javac.util.Names;
 import toolbox.JarTask;
 import toolbox.JavacTask;
 import toolbox.Task;
@@ -79,6 +84,37 @@
     }
 
     @Test
+    public void testNoDepNested(Path base) throws Exception {
+        Path src = base.resolve("src");
+        Path src_m1 = src.resolve("m1x");
+        tb.writeJavaFiles(src_m1,
+                          "module m1x { exports api; }",
+                          "package api; public class Api { public static class Nested {} }");
+        Path src_m2 = src.resolve("m2x");
+        tb.writeJavaFiles(src_m2,
+                          "module m2x { }",
+                          "package test; public class Test { api.Api.Nested nested; }");
+        Path classes = base.resolve("classes");
+        tb.createDirectories(classes);
+
+        List<String> log = new JavacTask(tb)
+                .options("-XDrawDiagnostics",
+                         "--module-source-path", src.toString())
+                .outdir(classes)
+                .files(findJavaFiles(src))
+                .run(Task.Expect.FAIL)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+
+        List<String> expected = Arrays.asList(
+                "Test.java:1:35: compiler.err.package.not.visible: api, (compiler.misc.not.def.access.does.not.read: m2x, api, m1x)",
+                "1 error");
+
+        if (!expected.equals(log))
+            throw new Exception("expected output not found; actual: " + log);
+    }
+
+    @Test
     public void testNotExported(Path base) throws Exception {
         Path src = base.resolve("src");
         Path src_m1 = src.resolve("m1x");
@@ -390,8 +426,7 @@
 
         List<String> expected = Arrays.asList(
                 "Test.java:1:22: compiler.err.package.not.visible: api, (compiler.misc.not.def.access.not.exported: api, m1x)",
-                "Test.java:1:49: compiler.err.not.def.access.package.cant.access: api.Api, api, (compiler.misc.not.def.access.not.exported: api, m1x)",
-                "2 errors");
+                "1 error");
 
         if (!expected.equals(log))
             throw new Exception("expected output not found; actual: " + log);
@@ -593,4 +628,80 @@
             throw new Exception("expected output not found; actual: " + log);
     }
 
+    @Test
+    public void testInaccessibleInSourceModuleViaBinaryModule(Path base) throws Exception {
+        Path src = base.resolve("src");
+        Path src_m1 = src.resolve("m1x");
+        tb.writeJavaFiles(src_m1,
+                          "@Deprecated module m1x { }");
+        Path src_m2 = src.resolve("m2x");
+        tb.writeJavaFiles(src_m2,
+                          "module m2x { requires transitive m1x; }");
+        Path src_m3 = src.resolve("m3x");
+        tb.writeJavaFiles(src_m3,
+                          "module m3x { requires transitive m2x; exports api; }",
+                          "package api; class Api { }");
+        Path classes = base.resolve("classes");
+        tb.createDirectories(classes);
+
+        new JavacTask(tb)
+            .options("-XDrawDiagnostics",
+                     "--module-source-path", src.toString())
+            .outdir(classes)
+            .files(findJavaFiles(src))
+            .run()
+            .writeAll();
+
+        tb.cleanDirectory(classes.resolve("m2x")); //force completion from source if needed
+        Files.delete(classes.resolve("m2x"));
+
+        tb.cleanDirectory(src_m3); //binary only module
+        Files.delete(src_m3);
+
+        //m4x does not depend on m1x/m2x/m3x, so cannot access api.Api
+        //but the recovery search should not complete m2x, as that would cause a deprecation warning:
+        Path src_m4 = src.resolve("m4x");
+        tb.writeJavaFiles(src_m4,
+                          "module m4x { }",
+                          "package m4x; public class Test extends api.Api { }");
+
+        List<String> log = new JavacTask(tb)
+                .options("-XDrawDiagnostics",
+                         "--module-source-path", src.toString(),
+                         "--module-path", classes.toString(),
+                         "-Xlint:deprecation")
+                .outdir(classes)
+                .files(findJavaFiles(src_m4))
+                .run(Task.Expect.FAIL)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+
+        List<String> expected = Arrays.asList(
+                "Test.java:1:40: compiler.err.package.not.visible: api, (compiler.misc.not.def.access.does.not.read: m4x, api, m3x)",
+                "1 error");
+
+        if (!expected.equals(log))
+            throw new Exception("expected output not found; actual: " + log);
+    }
+
+    @Test
+    public void testConvertNameCandidates(Path base) throws Exception {
+        Context ctx = new Context();
+        Names names = Names.instance(ctx);
+        Name name = names.fromString("com.sun.tools.javac.Attr.BreakAttr");
+
+        com.sun.tools.javac.util.List<String> actual =
+                Convert.classCandidates(name).map(n -> n.toString());
+        List<String> expected = Arrays.asList(
+                "com.sun$tools$javac$Attr$BreakAttr",
+                "com.sun.tools$javac$Attr$BreakAttr",
+                "com.sun.tools.javac$Attr$BreakAttr",
+                "com.sun.tools.javac.Attr$BreakAttr",
+                "com.sun.tools.javac.Attr.BreakAttr"
+        );
+
+        if (!expected.equals(actual)) {
+            throw new Exception("Expected names not generated: " + actual);
+        }
+    }
 }
--- a/langtools/test/tools/javac/modules/EdgeCases.java	Fri Jan 20 15:32:03 2017 +0100
+++ b/langtools/test/tools/javac/modules/EdgeCases.java	Fri Jan 20 15:32:07 2017 +0100
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8154283 8167320 8171098 8172809
+ * @bug 8154283 8167320 8171098 8172809 8173117
  * @summary tests for multi-module mode compilation
  * @library /tools/lib
  * @modules
@@ -496,6 +496,33 @@
     }
 
     @Test
+    public void testInvisibleClassVisiblePackageClash(Path base) throws Exception {
+        Path src = base.resolve("src");
+        Path src_m1 = src.resolve("m1x");
+        tb.writeJavaFiles(src_m1,
+                          "module m1x { }",
+                          "package m1x;\n" +
+                          "import m1x.a.*; public class Test { A a; }\n",
+                          "package m1x.a;\n" +
+                          "public class A { }\n");
+        Path src_m2 = src.resolve("m2x");
+        tb.writeJavaFiles(src_m2,
+                          "module m2x { }",
+                          "package m1x;\n" +
+                          "public class a { public static class A { } }\n");
+        Path classes = base.resolve("classes");
+        tb.createDirectories(classes);
+
+        new JavacTask(tb)
+            .options("--module-source-path", src.toString(),
+                     "-XDrawDiagnostics")
+            .outdir(classes)
+            .files(findJavaFiles(src))
+            .run()
+            .writeAll();
+    }
+
+    @Test
     public void testStripUnknownRequired(Path base) throws Exception {
         Path src = base.resolve("src");
         Path src_m1 = src.resolve("m1x");
--- a/langtools/test/tools/javac/modules/PackageMultipleModules.java	Fri Jan 20 15:32:03 2017 +0100
+++ b/langtools/test/tools/javac/modules/PackageMultipleModules.java	Fri Jan 20 15:32:07 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -70,9 +70,10 @@
                 .writeAll()
                 .getOutputLines(Task.OutputKind.DIRECT);
 
-        List<String> expected = Arrays.asList("A.java:1:22: compiler.err.package.not.visible: test, (compiler.misc.not.def.access.does.not.read: m1x, test, m2x)",
-                                              "B.java:1:22: compiler.err.package.not.visible: test, (compiler.misc.not.def.access.does.not.read: m2x, test, m1x)",
-                                              "2 errors");
+        List<String> expected = Arrays.asList(
+                "A.java:1:26: compiler.err.cant.resolve.location: kindname.class, B, , , (compiler.misc.location: kindname.package, test, null)",
+                "B.java:1:26: compiler.err.cant.resolve.location: kindname.class, A, , , (compiler.misc.location: kindname.package, test, null)",
+                "2 errors");
         if (!log.equals(expected))
             throw new Exception("expected output not found");
     }