From 1deafbc7ead6a93a4dd1ae5e6f98d8e846615cc0 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 18 Apr 2021 12:01:01 +0200 Subject: [PATCH 1/2] Allow export paths to see imports This simplifies the scoping rules. Fixes #12112 --- .../src/dotty/tools/dotc/typer/Namer.scala | 301 +++++++++--------- .../reference/other-new-features/export.md | 3 +- tests/pos/i12112.scala | 10 + 3 files changed, 166 insertions(+), 148 deletions(-) create mode 100644 tests/pos/i12112.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 3be5fae4df98..6d9d2114b234 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -972,162 +972,171 @@ class Namer { typer: Typer => def init(): Context = index(params) - /** Add forwarders as required by the export statements in this class */ - private def processExports(using Context): Unit = { + /** The forwarders defined by export `exp` */ + def exportForwarders(exp: Export)(using Context): List[tpd.MemberDef] = + val SKIP = "(skip)" // A string indicating that no forwarders for this kind of symbol are emitted + val buf = new mutable.ListBuffer[tpd.MemberDef] + val Export(expr, selectors) = exp + if expr.isEmpty then + report.error(em"Export selector must have prefix and `.`", exp.srcPos) + return Nil + + val path = typedAheadExpr(expr, AnySelectionProto) + checkLegalExportPath(path, selectors) + lazy val wildcardBound = importBound(selectors, isGiven = false) + lazy val givenBound = importBound(selectors, isGiven = true) + + def whyNoForwarder(mbr: SingleDenotation): String = { + val sym = mbr.symbol + if (!sym.isAccessibleFrom(path.tpe)) "is not accessible" + else if (sym.isConstructor || sym.is(ModuleClass) || sym.is(Bridge) || sym.is(ConstructorProxy)) SKIP + else if (cls.derivesFrom(sym.owner) && + (sym.owner == cls || !sym.is(Deferred))) i"is already a member of $cls" + else if (sym.is(Override)) + sym.allOverriddenSymbols.find( + other => cls.derivesFrom(other.owner) && !other.is(Deferred)) match { + case Some(other) => i"overrides ${other.showLocated}, which is already a member of $cls" + case None => "" + } + else "" + } - /** A string indicating that no forwarders for this kind of symbol are emitted */ - val SKIP = "(skip)" + /** Add a forwarder with name `alias` or its type name equivalent to `mbr`, + * provided `mbr` is accessible and of the right implicit/non-implicit kind. + */ + def addForwarder(alias: TermName, mbr: SingleDenotation, span: Span): Unit = + + def adaptForwarderParams(acc: List[List[tpd.Tree]], tp: Type, prefss: List[List[tpd.Tree]]) + : List[List[tpd.Tree]] = tp match + case mt: MethodType + if mt.paramInfos.nonEmpty && mt.paramInfos.last.isRepeatedParam => + // Note: in this branch we use the assumptions + // that `prefss.head` corresponds to `mt.paramInfos` and + // that `prefss.tail` corresponds to `mt.resType` + val init :+ vararg = prefss.head + val prefs = init :+ ctx.typeAssigner.seqToRepeated(vararg) + adaptForwarderParams(prefs :: acc, mt.resType, prefss.tail) + case mt: MethodOrPoly => + adaptForwarderParams(prefss.head :: acc, mt.resultType, prefss.tail) + case _ => + acc.reverse ::: prefss - /** The forwarders defined by export `exp`. - */ - def exportForwarders(exp: Export): List[tpd.MemberDef] = { - val buf = new mutable.ListBuffer[tpd.MemberDef] - val Export(expr, selectors) = exp - if expr.isEmpty then - report.error(em"Export selector must have prefix and `.`", exp.srcPos) - return Nil - - val path = typedAheadExpr(expr, AnySelectionProto) - checkLegalExportPath(path, selectors) - lazy val wildcardBound = importBound(selectors, isGiven = false) - lazy val givenBound = importBound(selectors, isGiven = true) - - def whyNoForwarder(mbr: SingleDenotation): String = { + if whyNoForwarder(mbr) == "" then val sym = mbr.symbol - if (!sym.isAccessibleFrom(path.tpe)) "is not accessible" - else if (sym.isConstructor || sym.is(ModuleClass) || sym.is(Bridge) || sym.is(ConstructorProxy)) SKIP - else if (cls.derivesFrom(sym.owner) && - (sym.owner == cls || !sym.is(Deferred))) i"is already a member of $cls" - else if (sym.is(Override)) - sym.allOverriddenSymbols.find( - other => cls.derivesFrom(other.owner) && !other.is(Deferred)) match { - case Some(other) => i"overrides ${other.showLocated}, which is already a member of $cls" - case None => "" + val forwarder = + if mbr.isType then + val forwarderName = checkNoConflict(alias.toTypeName, isPrivate = false, span) + var target = path.tpe.select(sym) + if target.typeParams.nonEmpty then + target = target.EtaExpand(target.typeParams) + newSymbol( + cls, forwarderName, + Exported | Final, + TypeAlias(target), + coord = span) + // Note: This will always create unparameterzied aliases. So even if the original type is + // a parameterized class, say `C[X]` the alias will read `type C = d.C`. We currently do + // allow such type aliases. If we forbid them at some point (requiring the referred type to be + // fully applied), we'd have to change the scheme here as well. + else { + def refersToPrivate(tp: Type): Boolean = tp match + case tp: TermRef => tp.termSymbol.is(Private) || refersToPrivate(tp.prefix) + case _ => false + val (maybeStable, mbrInfo) = + if sym.isStableMember && sym.isPublic && !refersToPrivate(path.tpe) then + (StableRealizable, ExprType(path.tpe.select(sym))) + else + (EmptyFlags, mbr.info.ensureMethodic) + var mbrFlags = Exported | Method | Final | maybeStable | sym.flags & RetainedExportFlags + if sym.is(ExtensionMethod) then mbrFlags |= ExtensionMethod + val forwarderName = checkNoConflict(alias, isPrivate = false, span) + newSymbol(cls, forwarderName, mbrFlags, mbrInfo, coord = span) } - else "" - } - - /** Add a forwarder with name `alias` or its type name equivalent to `mbr`, - * provided `mbr` is accessible and of the right implicit/non-implicit kind. - */ - def addForwarder(alias: TermName, mbr: SingleDenotation, span: Span): Unit = - - def adaptForwarderParams(acc: List[List[tpd.Tree]], tp: Type, prefss: List[List[tpd.Tree]]) - : List[List[tpd.Tree]] = tp match - case mt: MethodType - if mt.paramInfos.nonEmpty && mt.paramInfos.last.isRepeatedParam => - // Note: in this branch we use the assumptions - // that `prefss.head` corresponds to `mt.paramInfos` and - // that `prefss.tail` corresponds to `mt.resType` - val init :+ vararg = prefss.head - val prefs = init :+ ctx.typeAssigner.seqToRepeated(vararg) - adaptForwarderParams(prefs :: acc, mt.resType, prefss.tail) - case mt: MethodOrPoly => - adaptForwarderParams(prefss.head :: acc, mt.resultType, prefss.tail) - case _ => - acc.reverse ::: prefss - - if whyNoForwarder(mbr) == "" then - val sym = mbr.symbol - val forwarder = - if mbr.isType then - val forwarderName = checkNoConflict(alias.toTypeName, isPrivate = false, span) - var target = path.tpe.select(sym) - if target.typeParams.nonEmpty then - target = target.EtaExpand(target.typeParams) - newSymbol( - cls, forwarderName, - Exported | Final, - TypeAlias(target), - coord = span) - // Note: This will always create unparameterzied aliases. So even if the original type is - // a parameterized class, say `C[X]` the alias will read `type C = d.C`. We currently do - // allow such type aliases. If we forbid them at some point (requiring the referred type to be - // fully applied), we'd have to change the scheme here as well. - else { - def refersToPrivate(tp: Type): Boolean = tp match - case tp: TermRef => tp.termSymbol.is(Private) || refersToPrivate(tp.prefix) - case _ => false - val (maybeStable, mbrInfo) = - if sym.isStableMember && sym.isPublic && !refersToPrivate(path.tpe) then - (StableRealizable, ExprType(path.tpe.select(sym))) - else - (EmptyFlags, mbr.info.ensureMethodic) - var mbrFlags = Exported | Method | Final | maybeStable | sym.flags & RetainedExportFlags - if sym.is(ExtensionMethod) then mbrFlags |= ExtensionMethod - val forwarderName = checkNoConflict(alias, isPrivate = false, span) - newSymbol(cls, forwarderName, mbrFlags, mbrInfo, coord = span) - } - forwarder.info = avoidPrivateLeaks(forwarder) - forwarder.addAnnotations(sym.annotations) - val forwarderDef = - if (forwarder.isType) tpd.TypeDef(forwarder.asType) - else { - import tpd._ - val ref = path.select(sym.asTerm) - val ddef = tpd.DefDef(forwarder.asTerm, prefss => - ref.appliedToArgss(adaptForwarderParams(Nil, sym.info, prefss)) - ) - if forwarder.isInlineMethod then - PrepareInlineable.registerInlineInfo(forwarder, ddef.rhs) - ddef - } - - buf += forwarderDef.withSpan(span) - end addForwarder - - def addForwardersNamed(name: TermName, alias: TermName, span: Span): Unit = { - val size = buf.size - val mbrs = List(name, name.toTypeName).flatMap(path.tpe.member(_).alternatives) - mbrs.foreach(addForwarder(alias, _, span)) - if (buf.size == size) { - val reason = mbrs.map(whyNoForwarder).dropWhile(_ == SKIP) match { - case Nil => "" - case why :: _ => i"\n$path.$name cannot be exported because it $why" + forwarder.info = avoidPrivateLeaks(forwarder) + forwarder.addAnnotations(sym.annotations) + val forwarderDef = + if (forwarder.isType) tpd.TypeDef(forwarder.asType) + else { + import tpd._ + val ref = path.select(sym.asTerm) + val ddef = tpd.DefDef(forwarder.asTerm, prefss => + ref.appliedToArgss(adaptForwarderParams(Nil, sym.info, prefss)) + ) + if forwarder.isInlineMethod then + PrepareInlineable.registerInlineInfo(forwarder, ddef.rhs) + ddef } - report.error(i"""no eligible member $name at $path$reason""", ctx.source.atSpan(span)) + + buf += forwarderDef.withSpan(span) + end addForwarder + + def addForwardersNamed(name: TermName, alias: TermName, span: Span): Unit = { + val size = buf.size + val mbrs = List(name, name.toTypeName).flatMap(path.tpe.member(_).alternatives) + mbrs.foreach(addForwarder(alias, _, span)) + if (buf.size == size) { + val reason = mbrs.map(whyNoForwarder).dropWhile(_ == SKIP) match { + case Nil => "" + case why :: _ => i"\n$path.$name cannot be exported because it $why" } + report.error(i"""no eligible member $name at $path$reason""", ctx.source.atSpan(span)) } + } - def addWildcardForwardersNamed(name: TermName, span: Span): Unit = - List(name, name.toTypeName) - .flatMap(path.tpe.memberBasedOnFlags(_, excluded = Private|Given|ConstructorProxy).alternatives) - .foreach(addForwarder(name, _, span)) // ignore if any are not added - - def addWildcardForwarders(seen: List[TermName], span: Span): Unit = - val nonContextual = mutable.HashSet(seen: _*) - for mbr <- path.tpe.membersBasedOnFlags(required = EmptyFlags, excluded = PrivateOrSynthetic) do - if !mbr.symbol.isSuperAccessor then - // Scala 2 superaccessors have neither Synthetic nor Artfact set, so we - // need to filter them out here (by contrast, Scala 3 superaccessors are Artifacts) - val alias = mbr.name.toTermName - if mbr.symbol.is(Given) then - if !seen.contains(alias) && mbr.matchesImportBound(givenBound) then - addForwarder(alias, mbr, span) - else if !nonContextual.contains(alias) && mbr.matchesImportBound(wildcardBound) then - nonContextual += alias - addWildcardForwardersNamed(alias, span) - - def addForwarders(sels: List[untpd.ImportSelector], seen: List[TermName]): Unit = sels match - case sel :: sels1 => - if sel.isWildcard then - addWildcardForwarders(seen, sel.span) - else - if sel.rename != nme.WILDCARD then - addForwardersNamed(sel.name, sel.rename, sel.span) - addForwarders(sels1, sel.name :: seen) - case _ => + def addWildcardForwardersNamed(name: TermName, span: Span): Unit = + List(name, name.toTypeName) + .flatMap(path.tpe.memberBasedOnFlags(_, excluded = Private|Given|ConstructorProxy).alternatives) + .foreach(addForwarder(name, _, span)) // ignore if any are not added + + def addWildcardForwarders(seen: List[TermName], span: Span): Unit = + val nonContextual = mutable.HashSet(seen: _*) + for mbr <- path.tpe.membersBasedOnFlags(required = EmptyFlags, excluded = PrivateOrSynthetic) do + if !mbr.symbol.isSuperAccessor then + // Scala 2 superaccessors have neither Synthetic nor Artfact set, so we + // need to filter them out here (by contrast, Scala 3 superaccessors are Artifacts) + val alias = mbr.name.toTermName + if mbr.symbol.is(Given) then + if !seen.contains(alias) && mbr.matchesImportBound(givenBound) then + addForwarder(alias, mbr, span) + else if !nonContextual.contains(alias) && mbr.matchesImportBound(wildcardBound) then + nonContextual += alias + addWildcardForwardersNamed(alias, span) + + def addForwarders(sels: List[untpd.ImportSelector], seen: List[TermName]): Unit = sels match + case sel :: sels1 => + if sel.isWildcard then + addWildcardForwarders(seen, sel.span) + else + if sel.rename != nme.WILDCARD then + addForwardersNamed(sel.name, sel.rename, sel.span) + addForwarders(sels1, sel.name :: seen) + case _ => - addForwarders(selectors, Nil) - val forwarders = buf.toList - exp.pushAttachment(ExportForwarders, forwarders) - forwarders - } + addForwarders(selectors, Nil) + val forwarders = buf.toList + exp.pushAttachment(ExportForwarders, forwarders) + forwarders + end exportForwarders - for case exp @ Export(_, _) <- rest do - for forwarder <- exportForwarders(exp) do - forwarder.symbol.entered - } + /** Add forwarders as required by the export statements in this class */ + private def processExports(using Context): Unit = + + def process(stats: List[Tree])(using Context): Unit = stats match + case (stat: Export) :: stats1 => + for forwarder <- exportForwarders(stat) do + forwarder.symbol.entered + process(stats1) + case (stat: Import) :: stats1 => + process(stats1)(using ctx.importContext(stat, symbolOfTree(stat))) + case stat :: stats1 => + process(stats1) + case Nil => + + // Do a quick scan whether we need to process at all. This avoids creating + // import contexts for nothing. + if rest.exists(_.isInstanceOf[Export]) then + process(rest) + end processExports /** Ensure constructor is completed so that any parameter accessors * which have type trees deriving from its parameters can be diff --git a/docs/docs/reference/other-new-features/export.md b/docs/docs/reference/other-new-features/export.md index 5ded49aebc75..e493d7913a54 100644 --- a/docs/docs/reference/other-new-features/export.md +++ b/docs/docs/reference/other-new-features/export.md @@ -168,8 +168,7 @@ Export clauses are processed when the type information of the enclosing object o With export clauses, the following steps are added: - 6. Compute the types of all paths in export clauses in a context logically - inside the class but not considering any imports or exports in that class. + 6. Compute the types of all paths in export clauses. 7. Enter export aliases for the eligible members of all paths in export clauses. It is important that steps 6 and 7 are done in sequence: We first compute the types of _all_ diff --git a/tests/pos/i12112.scala b/tests/pos/i12112.scala new file mode 100644 index 000000000000..e979779353c0 --- /dev/null +++ b/tests/pos/i12112.scala @@ -0,0 +1,10 @@ +object A: + object B: + object C + +object X { + import A.B + + B.C // ok + export B.C // error +} From c16e41e2a2873809671c9154651dc063f068d94b Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 23 Apr 2021 17:14:32 +0200 Subject: [PATCH 2/2] Update compiler/src/dotty/tools/dotc/typer/Namer.scala Co-authored-by: Jamie Thompson --- compiler/src/dotty/tools/dotc/typer/Namer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 6d9d2114b234..10872e39b516 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -973,7 +973,7 @@ class Namer { typer: Typer => def init(): Context = index(params) /** The forwarders defined by export `exp` */ - def exportForwarders(exp: Export)(using Context): List[tpd.MemberDef] = + private def exportForwarders(exp: Export)(using Context): List[tpd.MemberDef] = val SKIP = "(skip)" // A string indicating that no forwarders for this kind of symbol are emitted val buf = new mutable.ListBuffer[tpd.MemberDef] val Export(expr, selectors) = exp