Use TypeError to improve type error messages. #265

judah · 2018-11-29T16:57:13Z

Adds an overlapping instance of HasField which gives
a better error message when no instance can be found.

Compare these results to:
https://gist.github.com/judah/e6678257cc28f2c486a631e49db885f2#file-lens-errors-md

New error message for the expression
def & value .~ 3 :: DescriptorProto :

<interactive>:6:14: error:
    • Type DescriptorProto has no field named "value"
    • In the first argument of ‘(.~)’, namely ‘#value’
      In the second argument of ‘(&)’, namely ‘#value .~ "hello"’
      In the expression:
          defMessage & #value .~ "hello" :: DescriptorProto

New error message for the expression () & #name .~ "hello":

<interactive>:6:6: error:
    • Type () has no field named "name"
    • In the first argument of ‘(.~)’, namely ‘#name’
      In the second argument of ‘(&)’, namely ‘#name .~ "hello"’
      In the expression: () & #name .~ "hello"
Previous error message:

In order for this improvement, needed to change the HasField' class slightly,
since overlapping instances don't work with fundeps (or, similarly, open type
families).

For example, previously we had:

class HasField' s (x :: Symbol) a | s x -> a where ...

instance HasField' Foo "fieldA" Int32 where ...
instance HasField' Foo "fieldB" Text where ...

In this change, we remove the fundep and move that constraint into
each instance:

class HasField' s (x :: Symbol) a where ...

instance a ~ Int32 => HasField' Foo "fieldA" where ...
instance a ~ Text => HasField' Foo "fieldB" a where ...

I don't think this is a big burden since all those instances are
generated automatically, and we also put in the Haddocks a more
human-readable list of fields for each type.

This change is

Adds an overlapping instance of `HasField` which gives a better error message when no instance can be found. Compare these results to: https://gist.github.com/judah/e6678257cc28f2c486a631e49db885f2#file-lens-errors-md New error message for the expression `def & value .~ 3 :: DescriptorProto` : ``` <interactive>:6:14: error: • Type DescriptorProto has no field named "value" • In the first argument of ‘(.~)’, namely ‘#value’ In the second argument of ‘(&)’, namely ‘#value .~ "hello"’ In the expression: defMessage & #value .~ "hello" :: DescriptorProto ``` New error message for the expression `() & #name .~ "hello"`: ``` <interactive>:6:6: error: • Type () has no field named "name" • In the first argument of ‘(.~)’, namely ‘#name’ In the second argument of ‘(&)’, namely ‘#name .~ "hello"’ In the expression: () & #name .~ "hello" Previous error message: ``` In order for this improvement, needed to change the HasField' class slightly, since overlapping instances don't work with fundeps (or, similarly, open type families). For example, previously we had: ``` class HasField' s (x :: Symbol) a | s x -> a where ... instance HasField' Foo "fieldA" Int32 where ... instance HasField' Foo "fieldB" Text where ... ``` In this change, we remove the fundep and move that constraint into each instance: ``` class HasField' s (x :: Symbol) a where ... instance a ~ Int32 => HasField' Foo "fieldA" where ... instance a ~ Text => HasField' Foo "fieldB" a where ... ``` I don't think this is a big burden since all those instances are generated automatically, and we also put in the Haddocks a more human-readable list of fields for each type.

judah · 2018-11-29T17:29:26Z

I'm not sure why CI is failing for the Cabal builds; maybe something changed in the PPA upstream? It seems unrelated to this change, though, since all the Stack builds are passing.

blackgnezdo · 2018-11-30T04:08:05Z

I'm not sure why CI is failing for the Cabal builds; maybe something changed in the PPA upstream? It seems unrelated to this change, though, since all the Stack builds are passing.

Agreed, the error is indicative of some infrastructure problem. My guess would be that apt-get suddenly became stricter and no longer allows sha1:

Reading package lists...
W: http://ppa.launchpad.net/couchdb/stable/ubuntu/dists/trusty/Release.gpg: Signature by key 15866BAFD9BCC4F3C1E0DFC7D69548E1C17EAB57 uses weak digest algorithm (SHA1)
The command "sudo -E apt-get -yq --no-install-suggests --no-install-recommends $(travis_apt_get_options) install cabal-install-2.0 ghc-8.2.1" failed and exited with 100 during .

blackgnezdo · 2018-11-30T04:09:53Z

lens-labels/src/Lens/Labels.hs

+-- be defined such that @a@ is derivable from @s@ and @x@.  For example, to define a
+-- field named @"x"@ of type @Int32@ for the containing type @Foo@:
+--
+-- > instance a ~ Int32 => HasLens' Foo "x" a where ...


Could you comment on why this is preferrable
instance a ~ Int32 => HasLens' Foo "x" a where ...
to this:
instance HasLens' Foo "x" Int32 where ...

I think I recognized the constraint trick, still makes sense to document it as such.

Added more explanation.

blackgnezdo · 2018-11-30T04:11:56Z

lens-labels/src/Lens/Labels.hs

 type ASetter s t a b = LensLike Identity s t a b

 (.~), set :: ASetter s t a b -> b -> s -> t
-f .~ x = f %~ const x
+f .~ x' = f %~ const x'


Why is this prime required? Did we magic up an x in global scope somehow?

Removed. (It was part of debugging that I forgot to remove.)

blackgnezdo · 2018-11-30T04:14:24Z

proto-lens-protoc/src/Data/ProtoLens/Compiler/Generate.hs

    --   lensOf _ = ...
    -- Note: for optional fields, this generates an instance both for "foo" and
    -- for "maybe'foo" (see plainRecordField below).
-    [ uncommented $ instDecl []
+    [ uncommented $ instDecl [equalP "a" (tyParen t)]


I think this results in a ~ Bar rather than Bar ~ a in the comment above. Make them consistent?

Good catch. Switched the comment.

blackgnezdo · 2018-12-02T02:46:21Z

lens-labels/src/Lens/Labels.hs

@@ -86,10 +86,13 @@ instance
 -- Note: in order to support better type error messages, this class does not have
 -- a fundep relationship to the parameter @a@.  Instead, instances are expected to
 -- be defined such that @a@ is derivable from @s@ and @x@.  For example, to define a
-- field named @"x"@ of type @Int32@ for the containing type @Foo@:
+-- field named @"r"@ of type @Int32@ for the containing type @Foo@, use:


Might as well give it a longer name to reduce the chance of collision?

This reverts commit 60b7c3c. It turned out that the TypeError would sometimes fire incorrectly, even when there was transitive instance available. Details: 4bb77a5 I suspect a GHC bug, but wasn't easily able to simplify the example enough to know for sure.

Adds an overlapping instance of `HasField` which gives a better error message when no instance can be found. Compare these results to: https://gist.github.com/judah/e6678257cc28f2c486a631e49db885f2#file-lens-errors-md New error message for the expression `def & value .~ 3 :: DescriptorProto` : ``` <interactive>:6:14: error: • Type DescriptorProto has no field named "value" • In the first argument of ‘(.~)’, namely ‘#value’ In the second argument of ‘(&)’, namely ‘#value .~ "hello"’ In the expression: defMessage & #value .~ "hello" :: DescriptorProto ``` New error message for the expression `() & #name .~ "hello"`: ``` <interactive>:6:6: error: • Type () has no field named "name" • In the first argument of ‘(.~)’, namely ‘#name’ In the second argument of ‘(&)’, namely ‘#name .~ "hello"’ In the expression: () & #name .~ "hello" Previous error message: ``` In order for this improvement, needed to change the HasField' class slightly, since overlapping instances don't work with fundeps (or, similarly, open type families). For example, previously we had: ``` class HasField' s (x :: Symbol) a | s x -> a where ... instance HasField' Foo "fieldA" Int32 where ... instance HasField' Foo "fieldB" Text where ... ``` In this change, we remove the fundep and move that constraint into each instance: ``` class HasField' s (x :: Symbol) a where ... instance a ~ Int32 => HasField' Foo "fieldA" where ... instance a ~ Text => HasField' Foo "fieldB" a where ... ``` I don't think this is a big burden since all those instances are generated automatically, and we also put in the Haddocks a more human-readable list of fields for each type.

…oogle#310) This reverts commit 60b7c3c. It turned out that the TypeError would sometimes fire incorrectly, even when there was transitive instance available. Details: google@4bb77a5 I suspect a GHC bug, but wasn't easily able to simplify the example enough to know for sure.

Adds an overlapping instance of `HasField` which gives a better error message when no instance can be found. Compare these results to: https://gist.github.com/judah/e6678257cc28f2c486a631e49db885f2#file-lens-errors-md New error message for the expression `def & value .~ 3 :: DescriptorProto` : ``` <interactive>:6:14: error: • Type DescriptorProto has no field named "value" • In the first argument of ‘(.~)’, namely ‘#value’ In the second argument of ‘(&)’, namely ‘#value .~ "hello"’ In the expression: defMessage & #value .~ "hello" :: DescriptorProto ``` New error message for the expression `() & #name .~ "hello"`: ``` <interactive>:6:6: error: • Type () has no field named "name" • In the first argument of ‘(.~)’, namely ‘#name’ In the second argument of ‘(&)’, namely ‘#name .~ "hello"’ In the expression: () & #name .~ "hello" Previous error message: ``` In order for this improvement, needed to change the HasField' class slightly, since overlapping instances don't work with fundeps (or, similarly, open type families). For example, previously we had: ``` class HasField' s (x :: Symbol) a | s x -> a where ... instance HasField' Foo "fieldA" Int32 where ... instance HasField' Foo "fieldB" Text where ... ``` In this change, we remove the fundep and move that constraint into each instance: ``` class HasField' s (x :: Symbol) a where ... instance a ~ Int32 => HasField' Foo "fieldA" where ... instance a ~ Text => HasField' Foo "fieldB" a where ... ``` I don't think this is a big burden since all those instances are generated automatically, and we also put in the Haddocks a more human-readable list of fields for each type.

…oogle#310) This reverts commit 60b7c3c. It turned out that the TypeError would sometimes fire incorrectly, even when there was transitive instance available. Details: google@4bb77a5 I suspect a GHC bug, but wasn't easily able to simplify the example enough to know for sure.

googlebot added the cla: yes label Nov 29, 2018

judah requested a review from blackgnezdo November 29, 2018 17:25

blackgnezdo approved these changes Nov 30, 2018

View reviewed changes

Fixes.

339e09c

blackgnezdo approved these changes Dec 2, 2018

View reviewed changes

judah merged commit 60b7c3c into google:master Dec 2, 2018

judah deleted the type-error-messages branch December 2, 2018 03:49

judah mentioned this pull request Feb 1, 2019

Rename lensOf to fieldOf, for consistency. #311

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use TypeError to improve type error messages. #265

Use TypeError to improve type error messages. #265

judah commented Nov 29, 2018 •

edited

Loading

judah commented Nov 29, 2018

blackgnezdo commented Nov 30, 2018

blackgnezdo Nov 30, 2018

blackgnezdo Dec 1, 2018

judah Dec 2, 2018

blackgnezdo Nov 30, 2018

judah Dec 2, 2018

blackgnezdo Nov 30, 2018

judah Dec 2, 2018

blackgnezdo Dec 2, 2018

Use TypeError to improve type error messages. #265

Use TypeError to improve type error messages. #265

Conversation

judah commented Nov 29, 2018 • edited Loading

judah commented Nov 29, 2018

blackgnezdo commented Nov 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

judah commented Nov 29, 2018 •

edited

Loading