Skip to content

Fix overly general typeclass in generated type annotations#133

Open
dillonkearns wants to merge 1 commit intomdgriffith:mainfrom
dillonkearns:fix/typeclass-constraints
Open

Fix overly general typeclass in generated type annotations#133
dillonkearns wants to merge 1 commit intomdgriffith:mainfrom
dillonkearns:fix/typeclass-constraints

Conversation

@dillonkearns
Copy link
Copy Markdown
Contributor

Functions using +, <, or ++ were incorrectly resulting in type annotations with plain type variables instead of constrained ones, producing code that doesn't compile.

Example

Elm.Declare.fn2 "addBoth"
    (Arg.var "a")
    (Arg.var "b")
    (\a b -> Elm.Op.plus a b)

Before (broken)

addBoth : a -> a -> a
addBoth a b =
    a + b

The Elm compiler rejects this:

-- TYPE MISMATCH

This `+` operator only works with `Int` and `Float` values.
But the type annotation says `a` which means ANY type.

After (fixed)

addBoth : number -> number -> number
addBoth a b =
    a + b

This applies to all three typeclass constraints:

Operator Before After
Elm.Op.plus a b a -> a -> a number -> number -> number
Elm.Op.lt a b a -> a -> Bool comparable -> comparable -> Bool
Elm.Op.append a b a -> a -> a appendable -> appendable -> appendable

When one operand is concrete, the constraint correctly narrows:

-- Elm.Op.plus a (Elm.float 1.0) generates:
addToFloat : Float -> Float -> Float

When a type variable stays polymorphic through an operator that
requires a typeclass constraint (+ requires number, < requires
comparable, ++ requires appendable), the constraint was lost during
type variable rewriting, producing `a -> a -> a` instead of
`number -> number -> number`.

Root cause: `resolveVariables` replaces constrained names (like
`number_0` or `comparable`) with arg variable names (like `arg_0`).
Then `rewriteTypeVariables` renames `arg_0` to `a`, losing the
constraint.

Fix: `rewriteTypeVariablesPreservingConstraints` builds a mapping
from resolved variable names back to their constraint names by
walking the inference cache bidirectionally. If a constrained name
maps to a generic (forward: `number_0 → arg_0`) or a generic maps
to a constrained name (reverse: `arg_0 → comparable`), the
constraint name is preserved during rewriting.

Before:
  Elm.Op.plus a b  →  addBoth : a -> a -> a
  Elm.Op.lt a b    →  compareBoth : a -> a -> Bool
  Elm.Op.append a b → appendBoth : a -> a -> a

After:
  Elm.Op.plus a b  →  addBoth : number -> number -> number
  Elm.Op.lt a b    →  compareBoth : comparable -> comparable -> Bool
  Elm.Op.append a b → appendBoth : appendable -> appendable -> appendable

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dillonkearns dillonkearns force-pushed the fix/typeclass-constraints branch from dee3310 to 2401bdf Compare April 2, 2026 23:41
Comment on lines +314 to +343
, test "number constraint with concrete: number -> number -> Float" <|
-- One arg is concrete (Float), the other stays polymorphic.
-- In Elm this is: number -> Float -> Float (the number unifies with Float)
\_ ->
Elm.Declare.fn2 "addToFloat"
(Arg.var "a")
(Arg.var "b")
(\a b -> Elm.Op.plus a (Elm.Op.plus b (Elm.float 1.0)))
|> .declaration
|> Elm.Expect.declarationAs
"""
addToFloat : Float -> Float -> Float
addToFloat a b =
a + (b + 1)
"""
, test "number constraint with one concrete Float arg" <|
-- When one arg is used with a Float literal, both args
-- resolve to Float (the number constraint narrows to Float)
\_ ->
Elm.Declare.fn2 "addToFloat"
(Arg.var "a")
(Arg.var "b")
(\a b -> Elm.Op.plus a (Elm.Op.plus b (Elm.float 1.0)))
|> .declaration
|> Elm.Expect.declarationAs
"""
addToFloat : Float -> Float -> Float
addToFloat a b =
a + (b + 1)
"""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be the same test twice? I'm guessing one of them was supposed to test (a + b) + 1?

@miniBill
Copy link
Copy Markdown
Contributor

miniBill commented Apr 6, 2026

Would love to see a compappend test too

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 this pull request may close these issues.

2 participants