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.

Sign up for GitHub

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

Jump to bottom

Fixes #1660 #1825

Merged
merged 10 commits into from
Jun 1, 2020
Merged

Fixes #1660 #1825

merged 10 commits into from
Jun 1, 2020

Conversation

mmhat
Copy link
Collaborator

@mmhat mmhat commented May 30, 2020

We do check for "simple" recursion, i.e.

data Foo = Foo Foo
    deriving Generic

instance FromDhall Foo

will result in an decoder throwing a RecursiveTypeError. Types like

data Foo = Foo Bar
    deriving Generic

instance FromDhall Foo

data Bar = Bar Foo
    deriving Generic

instance FromDhall Bar

will result in a non-terminating decoder like before.

Mann mit Hut added 7 commits May 30, 2020 17:20
Refactor ExtractErrors in order to reuse it with other errors
b6a3b19
Add error type suitable for Decoder.expected
5bb8567
Added test case for dhall-lang#1660
1b26f47
Extended ExtractError
2e94335
Decoder.expected can fail
30e3e96 
Changing type to Expector (Expr Src Void) which is
Validation (DhallErrors ExpectedTypeError) (Expr Src Void).
Check for simple recursive types
84293c2 
Simple recursive types are those where the type occurs in it's
definition.
Updated tutorial
c92aa35
@mmhat
Copy link
Collaborator Author

mmhat commented May 30, 2020

Since it's quiet a big change someone should have a short look at this.
Note: Encoder.declared is probably affected by this bug as well.

@mmhat mmhat requested a review from Gabriella439 May 30, 2020 18:04
@mmhat
Copy link
Collaborator Author

mmhat commented May 30, 2020

The Hydra CI fails, but it wasn't me:
dhall-lang/tests/type-inference/success/CacheImportsCanonicalize fails with 'Error: Remote host not found'.

@sjakobi sjakobi mentioned this pull request May 30, 2020
Open
@Gabriella439
Copy link
Collaborator

@mmhat: It looks like you will also need to add haddocks for these:

  98% (101 /103) in 'Dhall'
  Missing documentation for:
    ExpectedTypeError (src/Dhall.hs:263)
    showDhallErrors (src/Dhall.hs:217)

Gabriella439
Gabriella439 reviewed May 30, 2020
View reviewed changes
Copy link
Collaborator

@Gabriella439 Gabriella439 left a comment

Choose a reason for hiding this comment

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

Very nice work! Just a few small comments

dhall/src/Dhall.hs Outdated Show resolved Hide resolved
dhall/src/Dhall.hs Outdated Show resolved Hide resolved
dhall/src/Dhall.hs Outdated Show resolved Hide resolved
dhall/src/Dhall.hs Outdated Show resolved Hide resolved
dhall/src/Dhall.hs Outdated Show resolved Hide resolved
dhall/tests/Dhall/Test/Substitution.hs Outdated Show resolved Hide resolved
@Gabriella439 Gabriella439 merged commit 6bf8959 into dhall-lang:master Jun 1, 2020
@Gabriella439
Copy link
Collaborator

@mmhat: Thanks a lot for keeping at this and completing this! 🙂

@mmhat mmhat deleted the issue-1660-2 branch June 1, 2020 11:21
@mmhat mmhat mentioned this pull request Jun 1, 2020
Closed
@sjakobi sjakobi mentioned this pull request Jun 13, 2020
Closed
@paulyoung paulyoung mentioned this pull request Oct 7, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gabriella439 Gabriella439 Gabriella439 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mmhat @Gabriella439