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 initialization checker see through synthetic applys #14283

Merged
merged 7 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 17 additions & 5 deletions compiler/src/dotty/tools/dotc/transform/init/Semantic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -590,11 +590,20 @@ object Semantic {
def call(meth: Symbol, args: List[ArgInfo], superType: Type, source: Tree, needResolve: Boolean = true): Contextual[Result] = log("call " + meth.show + ", args = " + args, printer, (_: Result).show) {
def checkArgs = args.flatMap(_.promote)

def isSyntheticApply(meth: Symbol) =
meth.is(Flags.Synthetic)
&& meth.owner.is(Flags.Module)
&& meth.owner.companionClass.is(Flags.Case)

// fast track if the current object is already initialized
if promoted.isCurrentObjectPromoted then Result(Hot, Nil)
else value match {
case Hot =>
Result(Hot, checkArgs)
if isSyntheticApply(meth) then
val klass = meth.owner.companionClass.asClass
instantiate(klass, klass.primaryConstructor, args, source)
else
Result(Hot, checkArgs)

case Cold =>
val error = CallCold(meth, source, trace.toVector)
Expand All @@ -616,11 +625,14 @@ object Semantic {
given Trace = trace1
val cls = target.owner.enclosingClass.asClass
val ddef = target.defTree.asInstanceOf[DefDef]
val env2 = Env(ddef, args.map(_.value).widenArgs)
// normal method call
withEnv(if isLocal then env else Env.empty) {
eval(ddef.rhs, ref, cls, cacheResult = true) ++ checkArgs
}
if isSyntheticApply(meth) then
Copy link
Contributor

Choose a reason for hiding this comment

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

I wondered whether it would make sense to instantiate a synthetic apply even if target.hasSource is false. But if we don't have source for the synthetic apply, then we probably also don't have source for the class constructor, so instantiate wouldn't be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wondered whether it would make sense to instantiate a synthetic apply even if target.hasSource is false. But if we don't have source for the synthetic apply, then we probably also don't have source for the class constructor, so instantiate wouldn't be useful.

Yes, exactly.

I found one bug in the code above: the outer for instantiate is incorrect. I'll come up with a test to show the bug and fix it in this PR.

val klass = meth.owner.companionClass.asClass
instantiate(klass, klass.primaryConstructor, args, source)
else
withEnv(if isLocal then env else Env.empty) {
eval(ddef.rhs, ref, cls, cacheResult = true) ++ checkArgs
}
else if ref.canIgnoreMethodCall(target) then
Result(Hot, Nil)
else
Expand Down
8 changes: 0 additions & 8 deletions docs/docs/reference/other-new-features/safe-initialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,6 @@ With the established principles and design goals, following rules are imposed:
non-initialized object is not used, i.e. calling methods or accessing fields
on the escaped object is not allowed.

3. Local definitions may only refer to transitively initialized objects.

It means that in a local definition `val x: T = e`, the expression `e` may
only evaluate to transitively initialized objects. The same goes for local
lazy variables and methods. This rule is again motivated for simplicity in
reasoning about initialization: programmers may safely assume that all local
definitions only point to transitively initialized objects.

## Modularity

The analysis takes the primary constructor of concrete classes as entry points.
Expand Down
47 changes: 47 additions & 0 deletions tests/init/neg/apply.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
case class A(b: B)

object A:
def foo(b: B) = new A(b)
inline def bar(b: B) = new A(b)

class B:
val a = A(this)
val a2 = A.foo(this) // error
val a3 = A.bar(this)

// test receiver is ThisRef

object O:
case class A(b: B)

object A:
def foo(b: B) = new A(b)
inline def bar(b: B) = new A(b)

class B:
val a = A(this)
val a2 = A.foo(this) // error
val a3 = A.bar(this)

val b = new B
end O


// test receiver is Warm

class M(n: N):
case class A(b: B)

object A:
def foo(b: B) = new A(b)
inline def bar(b: B) = new A(b)

class B:
val a = A(this)
val a2 = A.foo(this) // error
val a3 = A.bar(this)
end M

class N:
val m = new M(this)
val b = new m.B