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

Extended error reporting #89

Closed
lykahb opened this issue Jun 20, 2014 · 3 comments · Fixed by #554
Closed

Extended error reporting #89

lykahb opened this issue Jun 20, 2014 · 3 comments · Fixed by #554

Comments

@lykahb
Copy link

lykahb commented Jun 20, 2014

The error function tries to mimic the original one by accepting only Symbols. This significantly constrains error reporting because on the type level there is no show function and symbol manipulation.

I suggest to make its argument kind-polymorphic so that we can pass details on the error site.

@amesgen
Copy link

amesgen commented Feb 13, 2023

This causes e.g. derived Enum instances to fail to compile with OverloadedStrings on GHC >=9.2 (probably due to sth like this?):

singletons [d|data Foo = Foo deriving (Enum)|]
    • Uninferrable type variable k10 in
      type family equation right-hand side:
      Apply
        @k10
        @Foo
        (ErrorSym0 @k10 @Foo)
        (FromString @k10 "toEnum: bad argument")
    • In the type family declaration for ‘Case_6989586621679042277’

Minimized, the issue is that this no longer compiles:

type family Foo (a :: ()) :: Bool where
  Foo '() = Error (FromString "bad")

(which makes sense, i.e. if the value-level error had type forall a b. a -> b, then error "a" will trigge -Wtype-defaults with OverloadedStrings).

Reverting 077aee5 fixes the error, but there are other conceivable fixes, but I am not even sure whether this should even be fixed; as I have no idea whether it is even expected/desired to ensure that all TH deriving also works in the presence of OverloadedStrings.

@RyanGlScott
Copy link
Collaborator

@amesgen, that is a very good observation. Indeed, the type Error (FromString "bad") is ambiguous, as it is not clear which PIsString instance is supposed to be used. I think it is reasonable to expect singletons [d|data Foo = Foo deriving (Enum)|] to work even with OverloadedStrings enabled, so I think it is worth reopening this issue.

While this issue predates me becoming a co-maintainer of the library, I can't help but wonder if the original motivation still holds true. The original motivation from #89 (comment) is:

The error function tries to mimic the original one by accepting only Symbols. This significantly constrains error reporting because on the type level there is no show function and symbol manipulation.

Nowadays, there is a type-level Show_ function, and the API for manipulating Symbols is nearly as expressive as the API for manipulating Strings. In light of this, I think a better design would be to:

  • Give Error the kind forall a. Symbol -> a.
  • Introduce a new PolyError type of kind forall a b. a -> b to replace the current, poly-kinded Error type.

Alternatively, we could keep Error's current kind and introduce a separate SymbolError :: forall a. Symbol -> a type, which we could use in place of Error in derived PEnum instances. That being said, I'm not as fond of this approach, which the issue lies in the fact that singletons-base treats Error differently than it does error, and I feel like Error is the thing that ought to change.

Thoughts on this, @lykahb?

@RyanGlScott RyanGlScott reopened this Feb 13, 2023
RyanGlScott added a commit that referenced this issue Feb 14, 2023
Previously, we had generalized the argument kind to `Error` in commit
077aee5 to permit passing things to `Error`
besides `Symbol`s. Back then, there was no way to `show` things at the type
level, nor was there a way to manipulate `Symbol`s in any meaningful fashion,
so this seemed like a reasonable choice.

Nowadays, however, the story is different. There is a type-level `Show_`
function, and the API for manipulating `Symbol`s is nearly as expressive as the
API for manipulating `String`s.  What's more, making `Error`'s argument kind
more general introduces ambiguity-related issues when deriving `Enum` instances
with `OverloadedStrings` enabled, as observed in #89.

In light of this, I have changed the API such that:

* The kind of the argument to `Error` (as well as the related
  `ErrorWithoutStackTrace` function) is now `Symbol`.  In this sense, this patch
  reverts 077aee5.
* There is now a new `Data.Singletons.Base.PolyError` module that provides a
  `PolyError` function. `PolyError` provides a kind-polymorphic `Error`
  interface much like what the previous type of `Error` was, so any existing
  code that relied on the argument of `Error` being kind-polymorphic can be
  migrated over to use `PolyError`.

Resolves #89 (hopefully for good this time).
RyanGlScott added a commit that referenced this issue Feb 16, 2023
Previously, we had generalized the argument kind to `Error` in commit
077aee5 to permit passing things to `Error`
besides `Symbol`s. Back then, there was no way to `show` things at the type
level, nor was there a way to manipulate `Symbol`s in any meaningful fashion,
so this seemed like a reasonable choice.

Nowadays, however, the story is different. There is a type-level `Show_`
function, and the API for manipulating `Symbol`s is nearly as expressive as the
API for manipulating `String`s.  What's more, making `Error`'s argument kind
more general introduces ambiguity-related issues when deriving `Enum` instances
with `OverloadedStrings` enabled, as observed in #89.

In light of this, I have changed the API such that:

* The kind of the argument to `Error` (as well as the related
  `ErrorWithoutStackTrace` function) is now `Symbol`.  In this sense, this patch
  reverts 077aee5.
* There is now a new `Data.Singletons.Base.PolyError` module that provides a
  `PolyError` function. `PolyError` provides a kind-polymorphic `Error`
  interface much like what the previous type of `Error` was, so any existing
  code that relied on the argument of `Error` being kind-polymorphic can be
  migrated over to use `PolyError`.

Resolves #89 (hopefully for good this time).
@RyanGlScott
Copy link
Collaborator

I've opened #554 with a patch that implements the PolyError plan.

RyanGlScott added a commit that referenced this issue Feb 18, 2023
Previously, we had generalized the argument kind to `Error` in commit
077aee5 to permit passing things to `Error`
besides `Symbol`s. Back then, there was no way to `show` things at the type
level, nor was there a way to manipulate `Symbol`s in any meaningful fashion,
so this seemed like a reasonable choice.

Nowadays, however, the story is different. There is a type-level `Show_`
function, and the API for manipulating `Symbol`s is nearly as expressive as the
API for manipulating `String`s.  What's more, making `Error`'s argument kind
more general introduces ambiguity-related issues when deriving `Enum` instances
with `OverloadedStrings` enabled, as observed in #89.

In light of this, I have changed the API such that:

* The kind of the argument to `Error` (as well as the related
  `ErrorWithoutStackTrace` function) is now `Symbol`.  In this sense, this patch
  reverts 077aee5.
* There is now a new `Data.Singletons.Base.PolyError` module that provides a
  `PolyError` function. `PolyError` provides a kind-polymorphic `Error`
  interface much like what the previous type of `Error` was, so any existing
  code that relied on the argument of `Error` being kind-polymorphic can be
  migrated over to use `PolyError`.

Resolves #89 (hopefully for good this time).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants