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

Do singled case expressions need an explicit kind annotation? #547

Closed
RyanGlScott opened this issue Dec 26, 2022 · 1 comment · Fixed by #549
Closed

Do singled case expressions need an explicit kind annotation? #547

RyanGlScott opened this issue Dec 26, 2022 · 1 comment · Fixed by #549

Comments

@RyanGlScott
Copy link
Collaborator

While pursuing a fix for #542, I uncovered this interesting piece of code:

singExp (ADCaseE exp matches ret_ty) res_ki =
-- See Note [Annotate case return type] and
-- Note [The id hack; or, how singletons-th learned to stop worrying and
-- avoid kind generalization]
DAppE (DAppTypeE (DVarE 'id)
(singFamily `DAppT` (ret_ty `maybeSigT` res_ki)))
<$> (DCaseE <$> singExp exp Nothing <*> mapM (singMatch res_ki) matches)

In particular, the part of interest is the ret_ty `maybeSigT` res_ki bit. Why do we need the explicit res_ki kind annotation? That is, why do we need id @(Sing (t :: k)) e instead of just id @(Sing t) e? This is what Note [Annotate case return type] has to say:

-- Note [Annotate case return type]
-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
-- We're straining GHC's type inference here. One particular trouble area
-- is determining the return type of a GADT pattern match. In general, GHC
-- cannot infer return types of GADT pattern matches because the return type
-- becomes "untouchable" in the case matches. See the OutsideIn paper. But,
-- during singletonization, we *know* the return type. So, just add a type
-- annotation. See #54.
--
-- In particular, we add a type annotation in a somewhat unorthodox fashion.
-- Instead of the usual `(x :: t)`, we use `id @t x`. See
-- Note [The id hack; or, how singletons-th learned to stop worrying and avoid
-- kind generalization] for an explanation of why we do this.

Curiously, this Note never actually discusses the kind annotation portion of the code. As an experiment, I decided to change ret_ty `maybeSigT` res_ki in the code to just ret_ty, and everything in singletons-base and its test suite continued to compile.

As historical context, this kind annotation was added in a60b000 in pursuit of a fix for #136. It's possible that this kind annotation was needed at the time, especially since case expressions were singled as e :: Sing (t :: k) rather than id @(Sing (t :: k)) e. This is because former interacts with kind generalization differently than the latter—see Note [The id hack; or, how singletons-th learned to stop worrying and avoid kind generalization] for the details. Now that singletons-th uses the id hack, it's quite possible that the kind annotation is no longer required due to the lack of kind generalization.

@RyanGlScott
Copy link
Collaborator Author

For some additional background on why I am interested in removing this kind annotation, consider this example:

absurd :: forall a. Void -> a
absurd v = case v of {}

Currently, this singles to something resembling:

sAbsurd :: forall a (v :: Void). Sing v -> Sing (Absurd v :: a)
sAbsurd (sV :: Sing v) = id @(Sing (Case v :: a)) (case sV of {})

Crucially, sAbsurd relies on a being in scope in id @(Sing (Case v :: a)). This is made possibly by a being quantified by an outermost forall. Without the outermost forall, GHC would reject sAbsurd.

Now consider what would happen in a variant of absurd that uses a nested forall:

absurd' :: Void -> forall a. a
absurd' v = case v of {}

If we singled absurd' naïvely using the approach outlined in #542, we'd get:

sAbsurd' :: forall (v :: Void). Sing v -> forall a. Sing (Absurd' v :: a)
sAbsurd' (sV :: Sing v) = id @(Sing (Case v :: a)) (case sV of {})

Unfortunately, this is not well scoped. Since the forall a. is not an outermost forall, the a in id @(Sing (Case v :: a)) is not in scope.

Possible ways to fix this include:

  1. Move the forall a to be on the outside in the type of sAbsurd'. While possible, I'd be sad to pick this option, as it would mean that the order of foralls would be substantially different between absurd' and sAbsurd'.

  2. Use the following trick to bring a into scope in the body of sAbsurd':

    sAbsurd' :: forall (v :: Void). Sing v -> forall a. Sing (Absurd' v :: a)
    sAbsurd' (sV :: Sing v) =
      (id @(Sing (Case v :: a)) (case sV of {}))
        :: forall a. Sing (Absurd' v :: a)

    This works, but figuring out how to generate this code automatically would be a bit tricky. In particular, we'd have to figure out which type variables to quantify. (Note that we wrote forall a. and not forall a v..)

  3. Remove the kind annotation entirely as described in the comment above. That way, a will never appear in the body of the function to begin with, which avoids the problem entirely. This seems like the simplest solution, so I'm inclined to go with this one unless there is some unforeseen difficulty with it.

RyanGlScott added a commit that referenced this issue Dec 26, 2022
… expressions

Explicit kind annotations aren't necessary in modern `singletons-th` due to the
approach that it uses for singling `case` expressions. This allows us to delete
a modest amount of code.

Fixes #547.
RyanGlScott added a commit that referenced this issue Dec 27, 2022
… expressions

Explicit kind annotations aren't necessary in modern `singletons-th` due to the
approach that it uses for singling `case` expressions. This allows us to delete
a modest amount of code.

Fixes #547.
RyanGlScott added a commit that referenced this issue Jan 2, 2023
… expressions

Explicit kind annotations aren't necessary in modern `singletons-th` due to the
approach that it uses for singling `case` expressions. This allows us to delete
a modest amount of code.

Fixes #547.
RyanGlScott added a commit that referenced this issue Jan 2, 2023
… expressions

Explicit kind annotations aren't necessary in modern `singletons-th` due to the
approach that it uses for singling `case` expressions. This allows us to delete
a modest amount of code.

Fixes #547.
RyanGlScott added a commit that referenced this issue May 19, 2024
Previously, `singletons-th` would always attempt to generate instance
signatures for singled instance methods, even if the original instance code
lacks instance signatures. To do so, `singletons-th` will infer an instance
signature by reifying the type of the method (or, if that cannot be found, the
singled version of the method) and manually applying a substitution from class
variables to instance types. This process is quite involved, and what's more,
it doesn't work in all cases:

* As noted in #358, inferred instance signatures can sometimes be ill-kinded.
* In order to support singling examples like the ones from #581, we need type
  variables from class method defaults and instance methods to scope over their
  bodies. However, the approach that `singletons-th` used to reify the method
  type for the singled code will sometimes reify _different_ type variables
  than the ones used in the promoted code, leading to disaster.

This convention of inferring the instance signature dates all the way back to
commit
c9beec5,
and it's unclear why this choice was made. At the time of writing #358, I was
convinced that inferred instance signatures were necessary to support examples
like the one in
#358 (comment).
However, this example is only problematic due to the use of an explicit kind
annotation on a promoted `case` expression, and these explicit kind annotations
were removed in the fix for #547. As such, this convention no longer serves a
useful purpose.

This patch removes the instance signature inference code, greatly simplifying
the overall process of singling instance declarations.

Fixes #590.
RyanGlScott added a commit that referenced this issue Jun 6, 2024
Previously, `singletons-th` would always attempt to generate instance
signatures for singled instance methods, even if the original instance code
lacks instance signatures. To do so, `singletons-th` will infer an instance
signature by reifying the type of the method (or, if that cannot be found, the
singled version of the method) and manually applying a substitution from class
variables to instance types. This process is quite involved, and what's more,
it doesn't work in all cases:

* As noted in #358, inferred instance signatures can sometimes be ill-kinded.
* In order to support singling examples like the ones from #581, we need type
  variables from class method defaults and instance methods to scope over their
  bodies. However, the approach that `singletons-th` used to reify the method
  type for the singled code will sometimes reify _different_ type variables
  than the ones used in the promoted code, leading to disaster.

This convention of inferring the instance signature dates all the way back to
commit
c9beec5,
and it's unclear why this choice was made. At the time of writing #358, I was
convinced that inferred instance signatures were necessary to support examples
like the one in
#358 (comment).
However, this example is only problematic due to the use of an explicit kind
annotation on a promoted `case` expression, and these explicit kind annotations
were removed in the fix for #547. As such, this convention no longer serves a
useful purpose.

This patch removes the instance signature inference code, greatly simplifying
the overall process of singling instance declarations.

Fixes #590.
RyanGlScott added a commit that referenced this issue Jun 6, 2024
Previously, `singletons-th` would always attempt to generate instance
signatures for singled instance methods, even if the original instance code
lacks instance signatures. To do so, `singletons-th` will infer an instance
signature by reifying the type of the method (or, if that cannot be found, the
singled version of the method) and manually applying a substitution from class
variables to instance types. This process is quite involved, and what's more,
it doesn't work in all cases:

* As noted in #358, inferred instance signatures can sometimes be ill-kinded.
* In order to support singling examples like the ones from #581, we need type
  variables from class method defaults and instance methods to scope over their
  bodies. However, the approach that `singletons-th` used to reify the method
  type for the singled code will sometimes reify _different_ type variables
  than the ones used in the promoted code, leading to disaster.

This convention of inferring the instance signature dates all the way back to
commit
c9beec5,
and it's unclear why this choice was made. At the time of writing #358, I was
convinced that inferred instance signatures were necessary to support examples
like the one in
#358 (comment).
However, this example is only problematic due to the use of an explicit kind
annotation on a promoted `case` expression, and these explicit kind annotations
were removed in the fix for #547. As such, this convention no longer serves a
useful purpose.

This patch removes the instance signature inference code, greatly simplifying
the overall process of singling instance declarations.

Fixes #590.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant