Add attached derive clauses to data and newtype declarations#4594
Add attached derive clauses to data and newtype declarations#4594i-am-the-slime wants to merge 8 commits into
Conversation
249ffef to
ca54e4a
Compare
85e4034 to
8bf8c27
Compare
8bf8c27 to
f973fdf
Compare
Adds Haskell-style derive clauses directly on data/newtype declarations:
data Color = Red | Green | Blue
derive (Eq, Ord)
newtype Name = Name String
derive newtype (Eq, Show)
Multiple classes per clause, multiple clauses per type, and free mixing
with standalone derive instance syntax. Automatically infers the correct
type application for higher-kinded classes like Functor.
f973fdf to
b056b2b
Compare
natefaubion
left a comment
There was a problem hiding this comment.
I don't find this particularly controversial, but it seems a little odd to have a different AST type just to run everything through TypeInstanceDeclaration anyway. It doesn't seem like you are utilizing this node in any useful way (such as for targeted errors). You should still have everything you need in Convert just to go ahead and generate a TypeInstanceDeclaration? It may be a little odd to have that desugaring in Convert, however.
Relying on the kinds in the environment works for these hard coded use cases, but I'm not sure how that works for subsequent features. It essentially means you can only derive things with this syntax as long as the class you want to derive is in an upstream module since this all happens during desugaring. This might be problematic for the more complicated extensions like newtype/via as you might want to derive a class in the same module as where you define it. Not necessarily a deal breaker for this as-is, but it seems like a big open question.
| -- A derive clause attached to a data or newtype declaration | ||
| -- (annotation, type name, type vars, class name) | ||
| -- | ||
| | DeriveClauseDeclaration SourceAnn (ProperName 'TypeName) [(Text, Maybe SourceType)] (Qualified (ProperName 'ClassName)) |
There was a problem hiding this comment.
Should this be tagged with a generated instance name? Both cases downstream are trying to generate instance names in two different ad hoc manners that aren't what's actually done in Convert.
| data DeriveClass a = DeriveClass | ||
| { dcAnn :: a | ||
| , dcClass :: QualifiedName (N.ProperName 'N.ClassName) | ||
| } deriving (Show, Eq, Ord, Functor, Foldable, Traversable, Generic) |
There was a problem hiding this comment.
I'm not totally sure why we need this, but the whole annotation thing in this CST is kind of bogus. I don't think the compiler uses annotations (the parser stashes () everywhere), it was mainly just a holdover from "maybe someone will need this" from when we tried to publish it as a stand alone library. There are no expected semantics, AFAIK. Honestly, I would just get rid of it and simplify this.
| Just pair -> pair | ||
| Nothing -> internalError "Empty class name" | ||
| tyConArgs <- computeInstArgs ss mn tyName tyVars mClassData className | ||
| go $ TypeInstanceDeclaration sa sa chainId 0 (Left genName) [] className tyConArgs DerivedInstance |
There was a problem hiding this comment.
Both the Newtype/Generic machinery and typeclasses here are doing work that is otherwise done in Convert for normal deriving (ie generating all the bookkeeping for TypeInstanceDeclaration. Should this all the relevant data just be in Convert and added to DeriveClauseDeclaration?
| data Either2 a b = Left2 a | Right2 b | ||
| derive (Bifunctor) | ||
|
|
||
| derive instance Eq a => Eq (Either2 a a) |
There was a problem hiding this comment.
We should probably have a failure test for something like this being derived?
Convert now emits TypeInstanceDeclaration directly with the bare type constructor as its single argument, matching what standalone `derive instance` produces. The intermediate DeriveClauseDeclaration node was constructed only to be rewritten in the desugarer, with the desugarer re-deriving facts (instance name, chain id, type-arg arity) that Convert already had. The arity computation in Sugar/TypeClasses, which looked up the class kind in the environment, also goes away. Kind matching is left to the typechecker, exactly as for standalone derive. As a side effect, the "can only derive upstream classes" limitation Nate flagged in review is gone. For Generic/Newtype attached deriving, binaryWildcardClass now accepts the bare-`[T]` form by looking up the type's vars and padding with the expected wildcard before falling into the existing handler.
Covers the case where the data type's kind doesn't fit the class — e.g. `data Box a derive (Eq)` produces `Eq Box` and the typechecker catches the mismatch (`Type -> Type` vs `Type`).
The CST annotation slot was always () — the parser stashed unit and nothing downstream read it. Without the parameter, both types lose their derived Functor/Foldable/Traversable too, which were equally unused.
Mirrors Nate's reference to `derive instance Eq a => Eq (Either2 a a)` on the standalone side: in attached form, `data Either2 a b derive (Eq)` emits `Eq Either2` and the typechecker rejects the kind mismatch (`Type -> Type -> Type` vs `Type`).
412d159 to
b3ddaa9
Compare
Yes, I did that on purpose, I think since I assumed we'd go with: newtype MyThing = MyThing String
derive (Eq, Ord)
derive newtype (Monoid)
derive (DecodeJSON) via (NonEmptyString)But maybe that's a bad idea. |
The earlier switch from --ghc-options -Werror to --haddock-arguments --optghc=-Werror broke CI on every push since 2026-04-25: it escalates warnings in transitive dependency Haddock builds (conduit, aeson, css-text, serialise, ...) to errors. The original comment warned against exactly this. Restore the local-packages-only -Werror.
Address PR review feedback: - Rename drvs to deriveClauses in CST.Convert for readability - Add failing test for combining derive (Newtype) with derive instance
Closes #3426.
Summary
Adds
deriveclauses directly on data/newtype declarations:Multiple classes per clause, multiple clauses per type, free mixing with standalone
derive instance.For higher-kinded classes, the number of type variables to apply is computed from the class's first parameter kind —
Functor/Foldable/Traversabledrop 1,Bifunctordrops 2, etc. No syntax is needed to specify it.Only natively derivable classes are supported in derive clauses. Non-derivable classes are rejected with a
CannotDeriveerror.This is a reduced version of #4592, scoping out
derive newtypeandderive viato keep the diff small.How it works
DeriveClass,DeriveClause) and grammar rulesDeriveClauseDeclarationper classTypeClasses.hsdesugars each into aTypeInstanceDeclarationwithDerivedInstancebody (the same nodederive instanceproduces) and hands it to the existing handlerkindArity— no per-class lookup table