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

singletonsOnly and fixity declarations for infix names don't play nicely #326

Closed
RyanGlScott opened this issue May 6, 2018 · 2 comments · Fixed by #430
Closed

singletonsOnly and fixity declarations for infix names don't play nicely #326

RyanGlScott opened this issue May 6, 2018 · 2 comments · Fixed by #430
Labels

Comments

@RyanGlScott
Copy link
Collaborator

While working on #323, I discovered something unsettling. The vast majority of infix type families that singletons produces, including ($), (++), (+), (-), (*), and many others, do not have fixity declarations! The culprit, as it turns out, is due to an unfortunate side-effect of the way singletons is architectured.

Using ($) an example, here is how we promote ($) currently:

$(singletonsOnly [d|
  ($) :: (a -> b) -> a -> b
  f $ x = f x
  infixr 0 $
  |])

Note that we use singletonsOnly because we don't want to generate a value-level ($). However, when processing the infixr 0 $ declaration, singletons notes that the promoted version of ($) is also ($), and thus generating a fixity declaration for the type-level ($) would be redundant, since there's already one in scope for the value-level ($)...

...well, except that's not true. We're using singletonsOnly, so we don't actually generate a value-level infixr 0 $ declaration! Thus, the type-level ($) is left without any fixity declaration at all. Major bummer.

One might wonder why singletonsOnly doesn't always generate fixity declarations. Alas, we can't due this, because it would break programs like this:

($) :: (a -> b) -> a -> b
f $ x = f x
infixr 0 $

$(singletonsOnly [d|
  ($) :: (a -> b) -> a -> b
  f $ x = f x
  infixr 0 $
  |])

Thus, singletons must be conservative here.

In an ideal world where we had the ability to control which namespace fixity declarations worked over, we could just write:

$(singletonsOnly [d|
  ($) :: (a -> b) -> a -> b
  f $ x = f x
  infixr value 0 $
  |])

And then we could always promote infixr value 0 $ to infixr type 0 $, without fear of name clashes. But we don't live in such a world today, so we'll have to think of a workaround in the interim. I can think of two workarounds:

  1. Replace all uses of singletonsOnly with singletons (making sure not to export the value-level identifiers that are generated).
  2. Manually insert fixity declarations for each promoting thing after the Template Haskell quotes.
@RyanGlScott RyanGlScott added the bug label May 6, 2018
RyanGlScott added a commit that referenced this issue May 6, 2018
This implements workaround (2) in
#326 (comment),
which has the advantages of:

(a) Not needing to generate a bunch of superfluous code.
(b) In the event that ghc-proposals/ghc-proposals#65
    is implemented, it will be much easier to notice that the hack can be
    removed (since the workaround will then produce an error).
@RyanGlScott
Copy link
Collaborator Author

I implemented workaround (2) in 40c736f, since:

(a) It doesn't require generating a bunch of superfluous code.
(b) In the event that ghc-proposals/ghc-proposals#65, is implemented, it will be much easier to notice that the hack can be removed (since the workaround will then produce an error).

@RyanGlScott
Copy link
Collaborator Author

RyanGlScott commented Jan 2, 2020

After implementing #427, I realized that there is a much cleaner way to resolve this issue than the hack I implemented in commit 40c736f. Currently, singletons and singletonsOnly are implemented in completely different ways, and there is no simple way for the internals of the TH machinery to know whether it is being invoked by singletons or singletonsOnly. But we could make this possible by adding genQuotedDecs :: Bool to Options and make the difference between singletons and singletonsOnly be whether genQuotedDecs is True or False. That way, promoteInfixDecl could know whether to generate fixity declarations for infix names by simply consulting the current genQuotedDecs setting.

I'll reopen this ticket as a reminder to implement this idea.

@RyanGlScott RyanGlScott reopened this Jan 2, 2020
RyanGlScott added a commit that referenced this issue Jan 3, 2020
This patch:

* Introduces a `genQuotedDecs :: Bool` option that controls whether
  the `q [Dec]` arguments in the `promote` and `singletons` functions
  should be generated as part of their output.
* Refactor `promote{Only}` and `singletons{Only}` to use
  `genQuotedDecs`.
* Changes `D.S.Promote.promoteInfixDecl` so that it _does_ promote
  fixity declarations for infix names when `genQuotedDecs` is
  `False`. (See the updated
  `Note [singletons and fixity declarations]` in `D.S.Single.Fixity`,
  wrinkle 1, for a more detailed explanation.) This fixes #326 in a
  much more robust way than the previous hacky workaround in commit
  40c736f.
RyanGlScott added a commit that referenced this issue Jan 4, 2020
This patch:

* Introduces a `genQuotedDecs :: Bool` option that controls whether
  the `q [Dec]` arguments in the `promote` and `singletons` functions
  should be generated as part of their output.
* Refactor `promote{Only}` and `singletons{Only}` to use
  `genQuotedDecs`.
* Changes `D.S.Promote.promoteInfixDecl` so that it _does_ promote
  fixity declarations for infix names when `genQuotedDecs` is
  `False`. (See the updated
  `Note [singletons and fixity declarations]` in `D.S.Single.Fixity`,
  wrinkle 1, for a more detailed explanation.) This fixes #326 in a
  much more robust way than the previous hacky workaround in commit
  40c736f.
RyanGlScott added a commit that referenced this issue Jan 13, 2020
…se (#430)

This patch:

* Introduces a `genQuotedDecs :: Bool` option that controls whether
  the `q [Dec]` arguments in the `promote` and `singletons` functions
  should be generated as part of their output.
* Refactor `promote{Only}` and `singletons{Only}` to use
  `genQuotedDecs`.
* Changes `D.S.Promote.promoteInfixDecl` so that it _does_ promote
  fixity declarations for infix names when `genQuotedDecs` is
  `False`. (See the updated
  `Note [singletons and fixity declarations]` in `D.S.Single.Fixity`,
  wrinkle 1, for a more detailed explanation.) This fixes #326 in a
  much more robust way than the previous hacky workaround in commit
  40c736f.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant