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

Make toplevel opaque types transparent to the whole file #21530

Closed
wants to merge 1 commit into from
Closed
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
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1620,6 +1620,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling

def findEnclosingThis(moduleClass: Symbol, from: Symbol): Type =
if ((from.owner eq moduleClass) && from.isPackageObject && from.is(Opaque)) from.thisType
else if (from eq moduleClass.owner) && moduleClass.isTopLevelDefinitionsObject && moduleClass.is(Opaque) then moduleClass.thisType
Copy link
Member

Choose a reason for hiding this comment

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

Isn't that going to create a C$.this reference outside of C$? If yes, I don't think that's supposed to be possible/valid.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reference wasn't outside when the user wrote it. The compiler moved it so it's nested in a package object, but to the user the call site isn't from "outside".

Copy link
Member

@sjrd sjrd Sep 2, 2024

Choose a reason for hiding this comment

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

I understand, but still ... from an elaborated, TASTy-level point of view, this is invalid. A C.this reference has no meaning outside of C. This is worse than being "not accessible" because of protected or even "not visible" because of private; here it does not even exist.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd argue that C.this, where C is a singleton module is just an alternative to C.type. Which, btw, is what widenInferred does as a final cleanup.

Copy link
Member

Choose a reason for hiding this comment

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

I'd argue that C.this, where C is a singleton module is just an alternative to C.type.

It represents the same set of values at run-time, sure, but it's not valid everywhere C.type is valid.

Which, btw, is what widenInferred does as a final cleanup.

AFAICT, widentInferred turns pre.C$ into pre.C; not C$.this into pre.C nor the converse.

else if (from.is(Package)) tp
else if ((from eq moduleClass) && from.is(Opaque)) from.thisType
else if (from eq NoSymbol) tp
Expand Down
7 changes: 7 additions & 0 deletions tests/pos/i15050.ex1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import OpaqueBug.*
def g(n: Counter): Counter = n
object OpaqueBug:
opaque type Counter = Int
val initial: Counter = 42
def f(n: Int): Int = g(n) + initial
@main def run = println(f(21))
6 changes: 6 additions & 0 deletions tests/pos/i15050.ex2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
opaque type Counter = Int
def g(n: Counter): Counter = n
object OpaqueBug:
val initial: Counter = 42 // was error: Found: (42 : Int)
def f(n: Int): Int = g(n) + initial // was error: Found: (n : Int) Required: Counter
@main def run = println(f(21))
Loading