Skip to content

Fix parameter untupling #14816

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

Merged
merged 3 commits into from
Apr 8, 2022
Merged

Fix parameter untupling #14816

merged 3 commits into from
Apr 8, 2022

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 31, 2022

As a preliminary: #14783 shows an example where an implicit reference was generated that started in a type.
The backend then crashed since it could not emit instructions to load that reference. We
now detect such references when they are created and flag them as errors.

The main commit is to fix the issue that created the bad reference in the first place.
Make untupled parameters ValDefs instead of DefDefs so that their references are
stable paths. Add an additional check that untupled parameters don't come from
call-by-name parameters (in that case, ValDefs would be unsound).

I was trying hard to allow paramater untupling for cbn parameters, but this seems to be
very hard since parameter untupling happens as a desugaring step when the expected
parameter type is not yet known. And it seems really hard to change from stable to
unstable (or vice versa) later in the compilation pipleline without doing a lot of
re-checking.

Since untupled cbn parameters are a pretty extreme corner case anyway I think it's
OK to just disallow them.

Fixes #14783

scala#14783 shows an example where an implicit reference was generated that started in a type.
The backend then crashed since it could not emit instructions to load that reference. We
now detect such references when they are created and flag them as errors.

We need to separately fix the issue that created the bad reference in the first place.
Make untupled parameters ValDefs instead of DefDefs so that their references are
stable paths. Add an additional check that untupled parameters don't come from
call-by-name parameters (in that case, ValDefs would be unsound).

I was trying hard to allow paramater untupling for cbn parameters, but this seems to be
very hard since parameter untupling happens as a desugaring step when the expected
parameter type is not yet known. And it seems really hard to change from stable to
unstable (or vice versa) later in the compilation pipleline without doing a lot of
re-checking.

Since untupled cbn parameters are a pretty extreme corner case anyway I think it's
OK to just disallow them.
@odersky odersky changed the title Flag unloadable references as errors Fix parameter untupling Mar 31, 2022
@odersky odersky requested a review from nicolasstucki March 31, 2022 19:14
@odersky odersky merged commit d673b97 into scala:main Apr 8, 2022
@odersky odersky deleted the fix-14783 branch April 8, 2022 11:50
odersky added a commit to dotty-staging/dotty that referenced this pull request Jul 13, 2022
We sometimes create a dependent parameter reference p.X where
`p` is a path with a type that has a wildcard argument, e.g. `P[X >: L <: H].`
So far the denotation of `p.X` had as info the bounds with which `X` was
declared in `P`. Now it gets the actual parameter bounds instead.

Fixes scala#15652

scala#15652 started failing when tuple unpackings started to use `val`s instead of `def`s
in scala#14816. That caused dependent class parameter references to be created and uncovered
the problem.
Kordyjan pushed a commit to dotty-staging/dotty that referenced this pull request Jul 26, 2022
We sometimes create a dependent parameter reference p.X where
`p` is a path with a type that has a wildcard argument, e.g. `P[X >: L <: H].`
So far the denotation of `p.X` had as info the bounds with which `X` was
declared in `P`. Now it gets the actual parameter bounds instead.

Fixes scala#15652

scala#15652 started failing when tuple unpackings started to use `val`s instead of `def`s
in scala#14816. That caused dependent class parameter references to be created and uncovered
the problem.
Kordyjan pushed a commit to dotty-staging/dotty that referenced this pull request Jul 26, 2022
We sometimes create a dependent parameter reference p.X where
`p` is a path with a type that has a wildcard argument, e.g. `P[X >: L <: H].`
So far the denotation of `p.X` had as info the bounds with which `X` was
declared in `P`. Now it gets the actual parameter bounds instead.

Fixes scala#15652

scala#15652 started failing when tuple unpackings started to use `val`s instead of `def`s
in scala#14816. That caused dependent class parameter references to be created and uncovered
the problem.
odersky added a commit to dotty-staging/dotty that referenced this pull request Sep 27, 2022
Be still more careful when intersecting info and arguments of a class type parameter.
This is the latest installment of a never-ending story.

The problem is this: Given
```scala
class C[T >: L1 <: H1] { val x: T }
def f(): C[? >: L2 <: H2]
```
what is the type of `f().x`?

With capture conversion (an extremely tricky aspect of the type system forced on us
since Java does it), the type is something like `?1.T` where `?1` is a skolem variable
of type `C[? >: L2 <: H2]`. OK, but what is the underlying (widened) type of `?1.T`?

We used to say it's `C[T >: L1 <: H1]`. I.e. we forgot about the actual arguments.
But then we had to change untupling desugarings from defs to vals in scala#14816 to fix
scala#14783 and it turned out that was not good enough, we needed the information of the
actual arguments, i.e. the type should be `C[T >: L2 <: H2]. Then something else started
failing which relied on the formal arguiment bounds being preserved. So the new resolution
was that the type would be the intersection of the formal parameter bounds and the actual
bounds, i.e. `C[T >: L1 | L2 <: H1 & H2]`. Then there was a series of problems where _that_
failed, either because the parameter bound was F-bounded or because the intersection was an
empty type. The latest installment is that the parameter bounds refer to another parameter
in the same class, which requires a simultaneous substitution of all skolemized bounds for
all dependent parameter references in the parameter bounds. But that's impossible or at least
very hard to achieve since we are looking at the type of a single parameter after capture conversion here.
So the current solution is to also back out of the intersection if there are cross-parameter
references and to use just the argument bounds instead.
odersky added a commit that referenced this pull request Sep 27, 2022
…16112)

Be still more careful when intersecting info and arguments of a class
type parameter. This is the latest installment of a never-ending story.

The problem is this: Given
```scala
class C[T >: L1 <: H1] { val x: T }
def f(): C[? >: L2 <: H2]
```
what is the type of `f().x`?

With capture conversion (an extremely tricky aspect of the type system
forced on us since Java does it), the type is something like `?1.T`
where `?1` is a skolem variable of type `C[? >: L2 <: H2]`. OK, but what
is the underlying (widened) type of `?1.T`?

We used to say it's `C[T >: L1 <: H1]`. I.e. we forgot about the actual
arguments. But then we had to change untupling desugarings from defs to
vals in #14816 to fix #14783 and it turned out that was not good enough,
we needed the information of the actual arguments, i.e. the type should
be `C[T >: L2 <: H2]`. Then something else started failing which relied
on the formal arguiment bounds being preserved. So the new resolution
was that the type would be the intersection of the formal parameter
bounds and the actual bounds, i.e. `C[T >: L1 | L2 <: H1 & H2]`. Then
there was a series of problems where _that_ failed, either because the
parameter bound was F-bounded or because the intersection was an empty
type. The latest installment is that the parameter bounds refer to
another parameter in the same class, which requires a simultaneous
substitution of all skolemized bounds for all dependent parameter
references in the parameter bounds. But that's impossible or at least
very hard to achieve since we are looking at the type of a single
parameter after capture conversion here. So the current solution is to
also back out of the intersection if there are cross-parameter
references and to use just the argument bounds instead.
mpollmeier pushed a commit to mpollmeier/dotty that referenced this pull request Oct 16, 2022
Be still more careful when intersecting info and arguments of a class type parameter.
This is the latest installment of a never-ending story.

The problem is this: Given
```scala
class C[T >: L1 <: H1] { val x: T }
def f(): C[? >: L2 <: H2]
```
what is the type of `f().x`?

With capture conversion (an extremely tricky aspect of the type system forced on us
since Java does it), the type is something like `?1.T` where `?1` is a skolem variable
of type `C[? >: L2 <: H2]`. OK, but what is the underlying (widened) type of `?1.T`?

We used to say it's `C[T >: L1 <: H1]`. I.e. we forgot about the actual arguments.
But then we had to change untupling desugarings from defs to vals in scala#14816 to fix
scala#14783 and it turned out that was not good enough, we needed the information of the
actual arguments, i.e. the type should be `C[T >: L2 <: H2]. Then something else started
failing which relied on the formal arguiment bounds being preserved. So the new resolution
was that the type would be the intersection of the formal parameter bounds and the actual
bounds, i.e. `C[T >: L1 | L2 <: H1 & H2]`. Then there was a series of problems where _that_
failed, either because the parameter bound was F-bounded or because the intersection was an
empty type. The latest installment is that the parameter bounds refer to another parameter
in the same class, which requires a simultaneous substitution of all skolemized bounds for
all dependent parameter references in the parameter bounds. But that's impossible or at least
very hard to achieve since we are looking at the type of a single parameter after capture conversion here.
So the current solution is to also back out of the intersection if there are cross-parameter
references and to use just the argument bounds instead.
Kordyjan pushed a commit to dotty-staging/dotty that referenced this pull request Oct 17, 2022
Be still more careful when intersecting info and arguments of a class type parameter.
This is the latest installment of a never-ending story.

The problem is this: Given
```scala
class C[T >: L1 <: H1] { val x: T }
def f(): C[? >: L2 <: H2]
```
what is the type of `f().x`?

With capture conversion (an extremely tricky aspect of the type system forced on us
since Java does it), the type is something like `?1.T` where `?1` is a skolem variable
of type `C[? >: L2 <: H2]`. OK, but what is the underlying (widened) type of `?1.T`?

We used to say it's `C[T >: L1 <: H1]`. I.e. we forgot about the actual arguments.
But then we had to change untupling desugarings from defs to vals in scala#14816 to fix
scala#14783 and it turned out that was not good enough, we needed the information of the
actual arguments, i.e. the type should be `C[T >: L2 <: H2]. Then something else started
failing which relied on the formal arguiment bounds being preserved. So the new resolution
was that the type would be the intersection of the formal parameter bounds and the actual
bounds, i.e. `C[T >: L1 | L2 <: H1 & H2]`. Then there was a series of problems where _that_
failed, either because the parameter bound was F-bounded or because the intersection was an
empty type. The latest installment is that the parameter bounds refer to another parameter
in the same class, which requires a simultaneous substitution of all skolemized bounds for
all dependent parameter references in the parameter bounds. But that's impossible or at least
very hard to achieve since we are looking at the type of a single parameter after capture conversion here.
So the current solution is to also back out of the intersection if there are cross-parameter
references and to use just the argument bounds instead.
bishabosha pushed a commit to dotty-staging/dotty that referenced this pull request Oct 18, 2022
We sometimes create a dependent parameter reference p.X where
`p` is a path with a type that has a wildcard argument, e.g. `P[X >: L <: H].`
So far the denotation of `p.X` had as info the bounds with which `X` was
declared in `P`. Now it gets the actual parameter bounds instead.

Fixes scala#15652

scala#15652 started failing when tuple unpackings started to use `val`s instead of `def`s
in scala#14816. That caused dependent class parameter references to be created and uncovered
the problem.
@Kordyjan Kordyjan added this to the 3.2.0 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants