Skip to content

Fix 13777 - refine check for caching companion in sum mirror #14035

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 13, 2021
Merged

Fix 13777 - refine check for caching companion in sum mirror #14035

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2fe2f03
reproduce issue #13777
bishabosha Dec 3, 2021
e5bf22e
Tighten check to cache Mirror.Sum in companion
bishabosha Dec 3, 2021
c156627
clarify comment in useCompanionAsSumMirror
bishabosha Dec 13, 2021
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
13 changes: 10 additions & 3 deletions compiler/src/dotty/tools/dotc/transform/SymUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,14 @@ object SymUtils:
self.isCoDefinedGiven(res.typeSymbol)
self.isAllOf(Given | Method) && isCodefined(self.info)

def useCompanionAsMirror(using Context): Boolean = self.linkedClass.exists && !self.is(Scala2x)
def useCompanionAsSumMirror(using Context): Boolean =
self.linkedClass.exists
&& !self.is(Scala2x)
&& (
// self is from source, or companion is a subtype of Sum
Copy link
Member

Choose a reason for hiding this comment

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

It'd be good to explain why for things which are not from source we need the more precise check (i.e., the difference in code generation between scala 3.0 and 3.1).

Copy link
Member

Choose a reason for hiding this comment

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

It's more about the other way round: ideally we can just check is subclass (which is only true in scala 3.1+). But we call this check before that parent is added, for code in compilation, so have to handle that specially.

Feels like the two new ones would subsume !self.is(Scala2x), right? (minor)

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 have clarified the comment to explain why we check if its from source

self.isDefinedInCurrentRun
|| self.linkedClass.isSubClass(defn.Mirror_SumClass)
)

/** Is this a sealed class or trait for which a sum mirror is generated?
* It must satisfy the following conditions:
Expand All @@ -129,7 +136,7 @@ object SymUtils:
s"it is not an abstract class"
else {
val children = self.children
val companionMirror = self.useCompanionAsMirror
val companionMirror = self.useCompanionAsSumMirror
assert(!(companionMirror && (declScope ne self.linkedClass)))
def problem(child: Symbol) = {

Expand All @@ -144,7 +151,7 @@ object SymUtils:
val s = child.whyNotGenericProduct
if (s.isEmpty) s
else if (child.is(Sealed)) {
val s = child.whyNotGenericSum(if child.useCompanionAsMirror then child.linkedClass else ctx.owner)
val s = child.whyNotGenericSum(if child.useCompanionAsSumMirror then child.linkedClass else ctx.owner)
if (s.isEmpty) s
else i"its child $child is not a generic sum because $s"
} else i"its child $child is not a generic product because $s"
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Synthesizer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context):

private def sumMirror(mirroredType: Type, formal: Type, span: Span)(using Context): Tree =
val cls = mirroredType.classSymbol
val useCompanion = cls.useCompanionAsMirror
val useCompanion = cls.useCompanionAsSumMirror

if cls.isGenericSum(if useCompanion then cls.linkedClass else ctx.owner) then
val elemLabels = cls.children.map(c => ConstantType(Constant(c.name.toString)))
Expand Down