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.

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

Approximate MatchTypes with lub of case bodies, if non-recursive #19761

Merged
merged 7 commits into from
Mar 4, 2024
Merged

Approximate MatchTypes with lub of case bodies, if non-recursive #19761

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8f2b6cf
Approximate MatchTypes with lub of case bodies, if non-recursive
dwijnand Feb 22, 2024
c25edde
Lub MatchTypeTree case body types and store as the inferred bound
dwijnand Feb 26, 2024
55ecbfc
Use isProvisional instead of has LazyRef
dwijnand Feb 27, 2024
3419aae
Fix PlusTri in pos/13633
dwijnand Feb 27, 2024
2423198
Handle NoType in disjointnessBoundary
dwijnand Feb 27, 2024
db1dcdc
Fix run-macros/type-show
dwijnand Feb 28, 2024
d687dee
Rework ParamRef#underlying handling in disjointnessBoundary
dwijnand Mar 4, 2024
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
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2863,6 +2863,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
disjointnessBoundary(tp.effectiveBounds.hi)
case tp: ErrorType =>
defn.AnyType
case NoType => // ParamRef#superTypeNormalized can return NoType
Copy link
Member

Choose a reason for hiding this comment

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

That seems to be a problem with ParamRef#superTypeNormalized rather than with disjointnessBoundary.

Copy link
Member Author

Choose a reason for hiding this comment

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

What should superTypeNormalized do?

Copy link
Member

Choose a reason for hiding this comment

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

It should return the declared type of the corresponding param, which is supposed to be its underlying. I don't see a scenario in which superTypeNormalized should ever return NoType, except possibly for AnyKind itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently, it can happen that it is not initialised:
https://github.com/lampepfl/dotty/blob/e994cf06eeee23e8a55a086907957e43140f2112/compiler/src/dotty/tools/dotc/core/Types.scala#L4690-L4695

Copy link
Member

Choose a reason for hiding this comment

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

That sort of explains it, although it's still concerning that we're attempting to read it from provablyDisjoint when it hasn't been initialized yet. Looks like a time bomb waiting to explode later due to weird compilation order issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, and this is not the only case, I ran into a similar issue where normalisation relies on the underlying type, which is not always set as expected. For a TypeRef, it goes through the denotation info, of which the completion seemed a bit unpredictable to me (heavily influenced by small changes in where things happen to be reduced).

Also (maybe) unrelated the normalization depends on the gadt ctx which doesn't seem to be reliably populated when using NamedTypes.

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 guess I should've opted for AnyKind rather than Any, as the more conservative approach.

Copy link
Member

Choose a reason for hiding this comment

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

I would still be less uncomfortable if we specifically handled ParamRef to deal with its superTypeNormalized returning NoType; instead of having this "catch-all" NoType. That would limit the blast radius of this workaround. We should also clearly comment that it is a workaround.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing.

defn.AnyType
end disjointnessBoundary

(disjointnessBoundary(tp1), disjointnessBoundary(tp2)) match
Expand Down
10 changes: 9 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2373,7 +2373,15 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
report.error(MatchTypeScrutineeCannotBeHigherKinded(sel1Tpe), sel1.srcPos)
val pt1 = if (bound1.isEmpty) pt else bound1.tpe
val cases1 = tree.cases.mapconserve(typedTypeCase(_, sel1Tpe, pt1))
assignType(cpy.MatchTypeTree(tree)(bound1, sel1, cases1), bound1, sel1, cases1)
val bound2 = if tree.bound.isEmpty then
val lub = cases1.foldLeft(defn.NothingType: Type): (acc, case1) =>
if !acc.exists then NoType
else if case1.body.tpe.isProvisional then NoType
else acc | case1.body.tpe
if lub.exists then TypeTree(lub, inferred = true)
else bound1
else bound1
assignType(cpy.MatchTypeTree(tree)(bound2, sel1, cases1), bound2, sel1, cases1)
}

def typedByNameTypeTree(tree: untpd.ByNameTypeTree)(using Context): ByNameTypeTree = tree.result match
Expand Down
2 changes: 1 addition & 1 deletion tests/pos/13633.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ object Sums extends App:

type Reverse[A] = ReverseLoop[A, EmptyTuple]

type PlusTri[A, B, C] = (A, B, C) match
type PlusTri[A, B, C] <: Tuple = (A, B, C) match
case (false, false, false) => (false, false)
case (true, false, false) | (false, true, false) | (false, false, true) => (false, true)
case (true, true, false) | (true, false, true) | (false, true, true) => (true, false)
Expand Down
7 changes: 7 additions & 0 deletions tests/pos/Tuple.Drop.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import compiletime.ops.int.*

type Drop[T <: Tuple, N <: Int] <: Tuple = N match
case 0 => T
case S[n1] => T match
case EmptyTuple => EmptyTuple
case x *: xs => Drop[xs, n1]
7 changes: 7 additions & 0 deletions tests/pos/Tuple.Elem.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import compiletime.ops.int.*

type Elem[T <: Tuple, I <: Int] = T match
case h *: tail =>
I match
case 0 => h
case S[j] => Elem[tail, j]
11 changes: 11 additions & 0 deletions tests/pos/i19710.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import scala.util.NotGiven

type HasName1 = [n] =>> [x] =>> x match {
case n => true
case _ => false
}
@main def Test = {
summon[HasName1["foo"]["foo"] =:= true]
summon[NotGiven[HasName1["foo"]["bar"] =:= true]]
summon[Tuple.Filter[(1, "foo", 2, "bar"), HasName1["foo"]] =:= Tuple1["foo"]] // error
}
Loading