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

Resolve name when named imp is behind wild imps #21888

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 42 additions & 39 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -238,38 +238,40 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
!owner.isEmptyPackage || ctx.owner.enclosingPackageClass.isEmptyPackage
}

import BindingPrec.*
type Result = (Type, BindingPrec)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should use a boxed type here. findRefRecur is very hot, we should avoid allocating where we can.

val NoResult = (NoType, NothingBound)

/** Find the denotation of enclosing `name` in given context `ctx`.
* @param previous A denotation that was found in a more deeply nested scope,
* or else `NoDenotation` if nothing was found yet.
* @param prevPrec The binding precedence of the previous denotation,
* or else `nothingBound` if nothing was found yet.
* @param prevResult A type that was found in a more deeply nested scope,
* and its precedence, or NoResult if nothing was found yet.
* @param prevCtx The context of the previous denotation,
* or else `NoContext` if nothing was found yet.
*/
def findRefRecur(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type = {
import BindingPrec.*
def findRefRecur(prevResult: Result, prevCtx: Context)(using Context): Result = {
val (previous, prevPrec) = prevResult

/** Check that any previously found result from an inner context
* does properly shadow the new one from an outer context.
* @param found The newly found result
* @param newPrec Its precedence
* @param newResult The newly found type and its precedence.
* @param scala2pkg Special mode where we check members of the same package, but defined
* in different compilation units under Scala2. If set, and the
* previous and new contexts do not have the same scope, we select
* the previous (inner) definition. This models what scalac does.
*/
def checkNewOrShadowed(found: Type, newPrec: BindingPrec, scala2pkg: Boolean = false)(using Context): Type =
def checkNewOrShadowed(newResult: Result, scala2pkg: Boolean = false)(using Context): Result =
val (found, newPrec) = newResult
if !previous.exists || TypeComparer.isSameRef(previous, found) then
found
newResult
else if (prevCtx.scope eq ctx.scope) && newPrec.beats(prevPrec) then
// special cases: definitions beat imports, and named imports beat
// wildcard imports, provided both are in contexts with same scope
found
newResult
else
if !scala2pkg && !previous.isError && !found.isError then
fail(AmbiguousReference(name, newPrec, prevPrec, prevCtx,
isExtension = previous.termSymbol.is(ExtensionMethod) && found.termSymbol.is(ExtensionMethod)))
previous
prevResult

/** Assemble and check alternatives to an imported reference. This implies:
* - If we expand an extension method (i.e. altImports != null),
Expand All @@ -282,12 +284,13 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
* shadowed. This order of checking is necessary since an outer package-level
* definition might trump two conflicting inner imports, so no error should be
* issued in that case. See i7876.scala.
* @param previous the previously found reference (which is an import)
* @param prevPrec the precedence of the reference (either NamedImport or WildImport)
* @param prevResult the previously found reference (which is an import), and
* the precedence of the reference (either NamedImport or WildImport)
* @param prevCtx the context in which the reference was found
* @param using_Context the outer context of `precCtx`
*/
def checkImportAlternatives(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type =
def checkImportAlternatives(prevResult: Result, prevCtx: Context)(using Context): Result =
val (previous, prevPrec) = prevResult

def addAltImport(altImp: TermRef) =
if !TypeComparer.isSameRef(previous, altImp)
Expand All @@ -302,20 +305,20 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
if prevPrec == WildImport then
// Discard all previously found references and continue with `altImp`
altImports.clear()
checkImportAlternatives(altImp, NamedImport, ctx)(using ctx.outer)
checkImportAlternatives((altImp, NamedImport), ctx)(using ctx.outer)
else
addAltImport(altImp)
checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
checkImportAlternatives(prevResult, prevCtx)(using ctx.outer)
case _ =>
if prevPrec == WildImport then
wildImportRef(curImport) match
case altImp: TermRef => addAltImport(altImp)
case _ =>
checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
checkImportAlternatives(prevResult, prevCtx)(using ctx.outer)
else
val found = findRefRecur(previous, prevPrec, prevCtx)
if found eq previous then checkNewOrShadowed(found, prevPrec)(using prevCtx)
else found
val foundResult = findRefRecur(prevResult, prevCtx)
if foundResult._1 eq previous then checkNewOrShadowed(foundResult)(using prevCtx)
else foundResult
end checkImportAlternatives

def selection(imp: ImportInfo, name: Name, checkBounds: Boolean): Type =
Expand Down Expand Up @@ -405,10 +408,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
!noImports &&
(prevPrec.ordinal < prec.ordinal || prevPrec == prec && (prevCtx.scope eq ctx.scope))

@tailrec def loop(lastCtx: Context)(using Context): Type =
if (ctx.scope eq EmptyScope) previous
@tailrec def loop(lastCtx: Context)(using Context): Result =
if (ctx.scope eq EmptyScope) prevResult
else {
var result: Type = NoType
var result: Result = NoResult
val curOwner = ctx.owner

/** Is curOwner a package object that should be skipped?
Expand Down Expand Up @@ -507,37 +510,36 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
effectiveOwner.thisType.select(name, defDenot)
}
if !curOwner.is(Package) || isDefinedInCurrentUnit(defDenot) then
result = checkNewOrShadowed(found, Definition) // no need to go further out, we found highest prec entry
result = checkNewOrShadowed((found, Definition)) // no need to go further out, we found highest prec entry
found match
case found: NamedType
if curOwner.isClass && isInherited(found.denot) && !ctx.compilationUnit.isJava =>
checkNoOuterDefs(found.denot, ctx, ctx)
case _ =>
else
if migrateTo3 && !foundUnderScala2.exists then
foundUnderScala2 = checkNewOrShadowed(found, Definition, scala2pkg = true)
foundUnderScala2 = checkNewOrShadowed((found, Definition), scala2pkg = true)._1
if (defDenot.symbol.is(Package))
result = checkNewOrShadowed(previous orElse found, PackageClause)
result = checkNewOrShadowed((previous orElse found, PackageClause))
else if (prevPrec.ordinal < PackageClause.ordinal)
result = findRefRecur(found, PackageClause, ctx)(using ctx.outer)
result = findRefRecur((found, PackageClause), ctx)(using ctx.outer)
}

if result.exists then result
if result._1.exists then result
else { // find import
val outer = ctx.outer
val curImport = ctx.importInfo
def updateUnimported() =
if (curImport.nn.unimported ne NoSymbol) unimported += curImport.nn.unimported
if (curOwner.is(Package) && curImport != null && curImport.isRootImport && previous.exists)
previous // no more conflicts possible in this case
else if (isPossibleImport(NamedImport) && (curImport nen outer.importInfo)) {
val namedImp = namedImportRef(curImport.uncheckedNN)
prevResult // no more conflicts possible in this case
else if (isPossibleImport(NamedImport) && curImport != null && (curImport ne outer.importInfo)) {
def updateUnimported() = if curImport.unimported ne NoSymbol then unimported += curImport.unimported
val namedImp = namedImportRef(curImport)
if (namedImp.exists)
checkImportAlternatives(namedImp, NamedImport, ctx)(using outer)
else if (isPossibleImport(WildImport) && !curImport.nn.importSym.isCompleting) {
val wildImp = wildImportRef(curImport.uncheckedNN)
checkImportAlternatives((namedImp, NamedImport), ctx)(using outer)
else if (isPossibleImport(WildImport) && !curImport.importSym.isCompleting) {
val wildImp = wildImportRef(curImport)
if (wildImp.exists)
checkImportAlternatives(wildImp, WildImport, ctx)(using outer)
checkImportAlternatives((wildImp, WildImport), ctx)(using outer)
else {
updateUnimported()
loop(ctx)(using outer)
Expand All @@ -556,7 +558,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
loop(NoContext)
}

findRefRecur(NoType, BindingPrec.NothingBound, NoContext)
val (foundRef, foundPrec) = findRefRecur(NoResult, NoContext)
foundRef
}

/** If `tree`'s type is a `TermRef` identified by flow typing to be non-null, then
Expand Down
9 changes: 9 additions & 0 deletions tests/pos/i18529/JCode1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package bug.code;

import bug.util.List;
import bug.util.*;
import java.util.*;

public class JCode1 {
public void m1(List<Integer> xs) { return; }
}
9 changes: 9 additions & 0 deletions tests/pos/i18529/JCode2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package bug.code;

import bug.util.*;
import bug.util.List;
import java.util.*;

public class JCode2 {
public void m1(List<Integer> xs) { return; }
}
3 changes: 3 additions & 0 deletions tests/pos/i18529/List.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package bug.util;

public final class List<E> {}
9 changes: 9 additions & 0 deletions tests/pos/i18529/SCode1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package bug.code

import bug.util.List
import bug.util.*
import java.util.*

class SCode1 {
def work(xs: List[Int]): Unit = {}
}
9 changes: 9 additions & 0 deletions tests/pos/i18529/SCode2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package bug.code

import bug.util.*
import bug.util.List
import java.util.*

class SCode2 {
def work(xs: List[Int]): Unit = {}
}
1 change: 1 addition & 0 deletions tests/pos/i18529/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class Test
Loading