template-haskell plainTV type inconsistency

[aavogt]

@yav pointed out this inconsistency here: https://gitlab.haskell.org/ghc/ghc/-/issues/17650

Here is a file using plainInvisTV

{-# LANGUAGE TemplateHaskell #-}
import Language.Haskell.TH.Lib.Internal
import Language.Haskell.TH (mkName)

f :: $(forallT [ plainInvisTV (mkName "n") inferredSpec ] (cxt []) [t| () |])
f = undefined

It does not work when the imports are changed:

{-# LANGUAGE TemplateHaskell #-}
import Language.Haskell.TH
-- or to Language.Haskell.TH.Lib

f :: $(forallT [ plainInvisTV (mkName "n") inferredSpec ] (cxt []) [t| () |])
f = undefined

t.hs:5:18: error:
    • Couldn't match type ‘Language.Haskell.TH.Syntax.TyVarBndr
                             Language.Haskell.TH.Syntax.Specificity’
                     with ‘Language.Haskell.TH.Syntax.Specificity’
      Expected: Language.Haskell.TH.Syntax.TyVarBndr
                  Language.Haskell.TH.Syntax.Specificity
        Actual: Language.Haskell.TH.Syntax.TyVarBndr
                  (Language.Haskell.TH.Syntax.TyVarBndr
                     Language.Haskell.TH.Syntax.Specificity)
    • In the expression: plainInvisTV (mkName "n") inferredSpec
      In the first argument of ‘forallT’, namely
        ‘[plainInvisTV (mkName "n") inferredSpec]’
      In the expression:
        forallT [plainInvisTV (mkName "n") inferredSpec] (cxt []) [t| () |]

The comment in TH/Lib.hs

but if a change occurs to Template
Haskell which requires breaking the API offered in this module, we opt to
copy the old definition here, and make the changes in
Language.Haskell.TH.Lib.Internal.

Should be changed to:

but if a change occurs to Template Haskell

1. When .Internal functions gain additional arguments, the
   function in .Lib will not.
2. Expressions that typecheck with .Internal also typecheck with .Lib,
   provided that any additional arguments are removed
3. In typical expressions such as `forallT [plainTV 'x] (cxt []) [t| Int |]`,
   intermediate types may change between template-haskell versions, but 
   the original expression will still typecheck if possible.

With template haskell 2.16 you could have forallT [plainTV _]. It doesn't work in 2.17 because forallT's type changed. If 2.17 had changed plainTV, perhaps to plainTV = plainInvisTV x specifiedSpec the original expression would still typecheck. In other words, "breaking the API" should read "typical expressions no longer typecheck" not "definitions in Lib.hs changed"

Likewise forallVisT :: Quote m => [m (TyVarBndr ())] -> m Type -> m Type should have been changed to

forallVisT :: Quote m => [m (TyVarBndr a)] -> m Type -> m Type, and then you could have only 4 definitions under type variable binders, namely plainTV, kindedTV, plainInferredTV, kindedInferredTV. If the .Internal continues to distinguish between TyVarBndr () and TyVarBndr Specificity, this part of the proposal prioritizes (3.) over (2.). Template haskell code that creates type variables could have been unaffected by "explicit specificity". Maybe it's not too late to go back for the next release.

[goldfirere]

I'm trying to figure out what you're asking for here. The statement above looks like you want a comment update within TH... which would seem more like a ticket appropriate to post for GHC. But, reading https://gitlab.haskell.org/ghc/ghc/-/issues/17650, there is a note toward the end requesting a discussion in this repo here to see whether we should scrap the stable Language.Haskell.TH.Lib and just have users use what is now Language.Haskell.TH.Lib.Internal directly. Is that what you're exploring? I'm just trying to figure out how to move this discussion forward -- thanks!

[aavogt]

Yes I opened this proposal here because of your comment in the ghc bug.

Besides keeping this difference:

Language.Haskell.TH.Lib.doE :: [m Stmt] -> m Exp
Language.Haskell.TH.Lib.Internal.doE -> Maybe ModName -> [m Stmt] -> m Exp
-- any others with different numbers of parameters

Language.Haskell.TH.Lib should export the current Language.Haskell.TH.Lib.Internal.

[goldfirere]

Thanks for clarifying. The title of this ticket seems very concerned specifically with plainTV, but it seems your aspirations are broader, no? And then your most recent comment suggests that you want to kill off Language.Haskell.TH.Lib functions in favor of their Language.Haskell.TH.Lib.Internal counterparts... except when the number of parameters differs? I'm open to these ideas -- I'm just trying to get a concrete sense of what you're asking for.

[aavogt]

After looking at the types between the two modules more closely, I propose the following:

Remove specifiedSpec, inferredSpec, plainInvisTV, kindedInvisTV and add or change the following:

-- or maybe use Monoid but I'm not sure about (<>) = max vs. min
-- to both .Lib and .Internal
class PossibleSpecificity a where
  possibleSpecificity :: Specificity
instance PossibleSpecificity () where
  possibleSpecificity = ()
instance PossibleSpecificity Specificity where
  possibleSpecificity = SpecifiedSpec

-- Language.Haskell.TH.Lib
plainTV :: PossibleSpecificity a=> Name -> TyVarBndr a
plainTV = PlainTV n possibleSpecificity
kindedTV :: PossibleSpecificity a => Name -> Kind -> TyVarBndr a
plainInferredTV :: Name -> TyVarBndr Specificity
kindedInferredTV :: Name -> TvVarBndr Specificity

-- Language.Haskell.TH.Lib.Internal
plainTV :: (Quote m, PossibleSpecificity a) => Name -> m (TyVarBndr a)
kindedTV :: (Quote m, PossibleSpecificity a) => Name -> m Kind -> m (TyVarBndr a)
plainInferredTV :: Name -> m (TyVarBndr Specificity)
kindedInferredTV :: Name -> m Kind -> m (TyVarBndr Specificity)

This follows the pattern already set by the treatment of TupleSections, QualifiedDo, ViewPatterns etc., users of Language.Haskell.TH.Lib do not have know about requesting the inferred specificity until they want it. In other words, forallT [plainTV _] will typecheck again. The explicit specificity proposal claims "This change is fully backward-compatible.", and the above makes that more likely for template haskell code.

Diff of Lib and Internal

I compare ghc -e ":browse Language.Haskell.TH.Lib" and ghc -e ":browse Language.Haskell.TH.Lib.Internal" below. It only shows how the Lib types are simpler and for that reason better.

m (TyVarBndr ()) / TyVrBndr ()

classD 
 Lib: m Cxt -> Name -> [TyVarBndr ()] -> [FunDep] -> [m Dec] -> m Dec
 Internal: m Cxt -> Name -> [m (TyVarBndr ())] -> [FunDep] -> [m Dec] -> m Dec

closedTypeFamilyD 
 Lib: -> [TyVarBndr ()]
 -> FamilyResultSig
 -> Maybe InjectivityAnn
 Internal: -> [m (TyVarBndr ())]
 -> m FamilyResultSig
 -> Maybe Language.Haskell.TH.Lib.Internal.InjectivityAnn

dataD 
 Lib: -> [TyVarBndr ()]
 -> Maybe Kind
 Internal: -> [m (TyVarBndr ())]
 -> Maybe (m Kind)

dataFamilyD 
 Lib:  Quote m => Name -> [TyVarBndr ()] -> Maybe Kind -> m Dec
 Internal:  Quote m => Name -> [m (TyVarBndr ())] -> Maybe (m Kind) -> m Dec

dataInstD 
 Lib: -> Name
 -> [m Type]
 -> Maybe Kind
 Internal: -> Maybe [m (TyVarBndr ())]
 -> m Type
 -> Maybe (m Kind)

forallC 
 Lib:  Quote m => [TyVarBndr Specificity] -> m Cxt -> m Con -> m Con
 Internal:  Quote m => [m (TyVarBndr Specificity)] -> m Cxt -> m Con -> m Con

forallT 
 Lib:  Quote m => [TyVarBndr Specificity] -> m Cxt -> m Type -> m Type
 Internal:  Quote m => [m (TyVarBndr Specificity)] -> m Cxt -> m Type -> m Type

kindedTV 
 Lib: Name -> Kind -> TyVarBndr ()
 Internal: Quote m => Name -> m Kind -> m (TyVarBndr ())

newtypeD 
 Lib: -> [TyVarBndr ()]
 -> Maybe Kind
 Internal: -> [m (TyVarBndr ())]
 -> Maybe (m Kind)

newtypeInstD 
 Lib: -> Name
 -> [m Type]
 -> Maybe Kind
 Internal: -> Maybe [m (TyVarBndr ())]
 -> m Type
 -> Maybe (m Kind)

openTypeFamilyD 
 Lib: -> [TyVarBndr ()]
 -> FamilyResultSig
 -> Maybe InjectivityAnn
 Internal: -> [m (TyVarBndr ())]
 -> m FamilyResultSig
 -> Maybe Language.Haskell.TH.Lib.Internal.InjectivityAnn

plainTV 
 Lib: Name -> TyVarBndr ()
 Internal: Quote m => Name -> m (TyVarBndr ())

pragRuleD 
 Lib: String -> [m RuleBndr] -> m Exp -> m Exp -> Phases -> m Dec
 Internal: String
 -> Maybe [m (TyVarBndr ())]
 -> [m RuleBndr]
 -> m Exp
 -> m Exp
 -> Phases
 -> m Dec

tySynD 
 Lib: Quote m => Name -> [TyVarBndr ()] -> m Type -> m Dec
 Internal: Quote m => Name -> [m (TyVarBndr ())] -> m Type -> m Dec

tySynEqn 
 Lib:  Quote m => Maybe [TyVarBndr ()] -> m Type -> m Type -> m TySynEqn
 Internal:  Quote m =>
 Maybe [m (TyVarBndr ())] -> m Type -> m Type -> m TySynEqn

tyVarSig 
 Lib: TyVarBndr () -> FamilyResultSig
 Internal: Quote m => m (TyVarBndr ()) -> m FamilyResultSig

These are missing from the diff:

plainInvisTV
 Lib: Quote m => Name -> Specificity -> TyVarBndr Specificity
 Internal: Quote m => Name -> Specificity -> m (TyVarBndr Specificity)

kindedInvisTV
 Lib: Quote m => Name -> Specificity -> Kind -> TyVarBndr Specificity
 Internal: Quote m => Name -> Specificity -> m Kind -> m (TyVarBndr Specificity)

The "missing diff" above is the smallest change that removes the inconsistency that caused the ghc bug referenced above. I don't think it needs any discussion here. Once those two functions are changed, the bug will be fixed, and then figuring out this proposal becomes less important. On the other hand, those two functions will be removed by my proposal.

Internal supports an extension

Keep these two versions. If the modules were merged, Internal. versions will be renamed to describe the extension. For example tupSectionE doQualE

doE 
 Lib: Quote m => [m Stmt] -> m Exp
 Internal: Quote m => Maybe ModName -> [m Stmt] -> m Exp

mdoE 
 Lib: Quote m => [m Stmt] -> m Exp
 Internal: Quote m => Maybe ModName -> [m Stmt] -> m Exp

tupE 
 Lib: Quote m => [m Exp] -> m Exp
 Internal: Quote m => [Maybe (m Exp)] -> m Exp

unboxedTupE 
 Lib: Quote m => [m Exp] -> m Exp
 Internal: Quote m => [Maybe (m Exp)] -> m Exp

m Kind / Kind

These could stay as-is.

constraintK 
 Lib: Kind
 Internal: Quote m => m Kind

sigT 
 Lib: Quote m => m Type -> Kind -> m Type
 Internal: Quote m => m Type -> m Kind -> m Type

starK 
 Lib: Kind
 Internal: Quote m => m Kind

m FamilyResultSig / FamilyResultSig

openTypeFamilyD 
 Lib: -> [TyVarBndr ()]
 -> FamilyResultSig
 -> Maybe InjectivityAnn
 Internal: -> [m (TyVarBndr ())]
 -> m FamilyResultSig
 -> Maybe Language.Haskell.TH.Lib.Internal.InjectivityAnn

kindSig 
 Lib: Kind -> FamilyResultSig
 Internal: Quote m => m Kind -> m FamilyResultSig

noSig 
 Lib: FamilyResultSig
 Internal: Quote m => m FamilyResultSig

openTypeFamilyD 
 Lib: -> [TyVarBndr ()]
 -> FamilyResultSig
 -> Maybe InjectivityAnn
 Internal: -> [m (TyVarBndr ())]
 -> m FamilyResultSig
 -> Maybe Language.Haskell.TH.Lib.Internal.InjectivityAnn

m DerivStrategy / DerivStrategy

derivClause 
 Lib:  Quote m => Maybe DerivStrategy -> [m Pred] -> m DerivClause
 Internal:  Quote m => Maybe (m DerivStrategy) -> [m Pred] -> m DerivClause

standaloneDerivWithStrategyD 
 Lib:  Quote m => Maybe DerivStrategy -> m Cxt -> m Type -> m Dec
 Internal:  Quote m => Maybe (m DerivStrategy) -> m Cxt -> m Type -> m Dec

data DerivClause 
 Lib: DerivClause (Maybe DerivStrategy) Cxt
 Internal: missing

data DerivStrategy
 Lib: StockStrategy
 | AnyclassStrategy
 | NewtypeStrategy
 | ViaStrategy Type
 Internal: missing

Here the very minor inconsistency is that Lib exports stockStrategy, anyclassStrategy, newtypeStrategy, viaStrategy :: Quote m => m DerivStrategy but can't consume those directly.

re-exports

Should be added to missing

data Overlap 
 Lib: Overlappable | Overlapping | Overlaps | Incoherent
 Internal: missing

mkBytes 
 Lib:  GHC.ForeignPtr.ForeignPtr GHC.Word.Word8 -> Word -> Word -> Bytes
 Internal: missing

-- deprecate and remove?
type Decs  Lib: missing Internal: [Dec]
 *

type Language.Haskell.TH.Lib.Internal.InjectivityAnn  Lib: missing Internal:  Language.Haskell.TH.Syntax.InjectivityAnn
 *

type Language.Haskell.TH.Lib.Internal.Role  Lib: missing Internal:  Language.Haskell.TH.Syntax.Role
 *

[phadej]

Wasn't the idea of this issue to take a poll, and not to discuss things further. Now we have discussion in two places. Bad.

EDIT: I think that the people who deal with TH (and its issues) follow the GHC issue tracker more closely then ghc-proposals repository.

[goldfirere]

I suggested moving the discussion here thinking it would get more eyes here than in a GHC ticket. Perhaps I'm wrong. It's hard getting the right information in front of the right eyes!

[goldfirere]

Thanks @aavogt for the detailed post with all the diffs. That's helpful seeing how the types differ between the two modules.

However, I'm still not seeing exactly what you are proposing. The top of your post includes "I propose the following:" with a list of functions to remove and a bunch of type declarations. This is good... but still not completely clear to me: are you proposing these changes for .Lib? or for .Internal? or both?

But after this proposal, there are lots of other diffs of types. Are these just background information? Are they part of the proposal? I'm not sure.

There is a tantalizing "Is the smallest change that removes the inconsistency" -- but there is no subject of the sentence! What is that smallest change? Note that the GHC bug describes a design infelicity that was introduced on purpose, so it's not clear to me that "fixing" it will benefit everyone.

The "Internal supports an extension" section sounds like it's an alternative to the original proposal? I'm not sure how it relates to the rest of the post.

Maybe the best way forward is actually to prepare your dream MR against these files in the GHC repo and then link to this MR here?

Sorry to pick apart your words -- I really do want to have an opinion here, but I can't without knowing what we're discussing. Thanks for the time you're putting in!

[phadej]

IMO, best forum for library issues is still libraries@haskell.org, especially if you want to reach the users of base or template-haskell, then also post links on twitter, reddit, haskell discourse. (Compare to how many replies from the different people the Eq issue got already today on the mailing list!)

[aavogt]

But after this proposal, there are lots of other diffs of types. Are these just background information?

It is just background information. There are only a few ways in which the modules differ. Functions under each heading differ in the same way. Sometimes a function such as openTypeFamilyD should be under two headings, but I did not do that for each section.

There is a tantalizing "Is the smallest change that removes the inconsistency" -- but there is no subject of the sentence! What is that smallest change?

I edited this part.

[goldfirere]

This is becoming clearer, but it's still hazy for me.

It seems that you're actually proposing three different alternatives: one at the top, the smallest change that removes the inconsistency, and then the "Internal supports an extension" piece. Is that right? Are these all alternatives? Do they achieve the same goals?

Then there is the final section about re-exports, but I don't see yet what's going on there or how that section relates to the rest of the proposal. What re-exports are you adding to what module? I'm not sure of the notation conventions in the code block there.

However, I'm also wary that I keep asking for more of your time, and it's quite possible that, at the end of the day, when I understand all of this, we'll all think it's not the right way forward. I don't really know how to avoid this -- I still feel like I don't understand enough of what you're suggesting to have an opinion -- but I don't want to waste your time. The top proposal is clear to me... but its effects are not. That is, I see what you want to change. But I don't fully understand why or how it will affect others. Maybe the biggest question is: how will this proposal affect the way TH evolves in the future? That is, we have a policy right now of keeping .Lib fixed and changing only .Internal as needed. Maybe you're suggesting a change to this policy (and then some quick catching up in .Lib)? Figuring out the new policy and its effects would, I think, lead to a better understanding of this current proposal.