Support new JSON format #42

paulyoung · 2018-02-15T07:25:03Z

Should close #41.

This is a port of FromJSON.hs. ~~I still need to port TestCoreFn.hs.~~

paulyoung · 2018-02-15T08:15:17Z

src/CoreFn/Expr.purs

+-- instance functorCaseAlternative :: Functor CaseAlternative where
+--   map f (CaseAlternative cabs car) = CaseAlternative
+--     (map (map f) cabs)
+--     (either (Left <<< map (map f *** map f)) (Right <<< map f) car)


I still need to figure out how to satisfy the Functor instances that this depends on.

While that either call is concise, I find it difficult to parse. A case expression might be better if you plan on having less experienced contributors in the future.

I don’t disagree but I tried to keep this port as close as possible to the Haskell version so that nothing was accidentally lost in translation.

https://github.com/purescript/purescript/blob/f999179792ae2c49b2ad3019959081540da0ad9f/src/Language/PureScript/CoreFn/Expr.hs#L89-L93

Got it. That explains the style. Thanks for the explanation and working on the port. It'll make adding backends easier in the future.

paulyoung · 2018-02-22T08:23:26Z

The tests include the JSON output from running stack test --test-arguments="-p corefn" on a modified TestCoreFn.hs, with the following diff applied:

diff --git a/tests/TestCoreFn.hs b/tests/TestCoreFn.hs
index 3f6972b1..5b0f0a79 100644
--- a/tests/TestCoreFn.hs
+++ b/tests/TestCoreFn.hs
@@ -12,6 +12,9 @@ import Data.Aeson
 import Data.Aeson.Types
 import Data.Version

+import Data.ByteString.Lazy.Char8
+import Debug.Trace
+
 import Language.PureScript.AST.Literals
 import Language.PureScript.AST.SourcePos
 import Language.PureScript.Comments
@@ -34,7 +37,8 @@ parseModule = parse moduleFromJSON
 parseMod :: Module Ann -> Result (Module Ann)
 parseMod m =
   let v = Version [0] []
-  in snd <$> parseModule (moduleToJSON v m)
+      j = moduleToJSON v m
+  in snd <$> parseModule (trace (unpack (encode j)) j)

 isSuccess :: Result a -> Bool
 isSuccess (Success _) = True

paulyoung · 2018-02-22T08:46:18Z

src/CoreFn/Expr.purs

+--   map f (CaseAlternative { caseAlternativeBinders, caseAlternativeResult }) = CaseAlternative
+--     { caseAlternativeBinders: (map (map f) caseAlternativeBinders)
+--     , caseAlternativeResult: (either (Left <<< map (map f *** map f)) (Right <<< map f) caseAlternativeResult)
+--     }


I think the last thing to do in this PR is to figure out why the Functor instances that are commented out don't work.

They're not actually used anywhere but it might be nice to have them.

This was taken from https://github.com/purescript/purescript/blob/c19a79285c5697b59c46803483b41ffeaa5cd7b0/src/Language/PureScript/CoreFn/Expr.hs#L89-L93

I don't see a Functor instance for Expr.

The other instances are here:

purescript-corefn/src/CoreFn/Expr.purs

Line 65 in aa1d736

-- derive instance functorExpr :: Functor Expr

purescript-corefn/src/CoreFn/Expr.purs

Line 117 in aa1d736

-- derive instance functorBind :: Functor Bind

This is the error:

[1/1 TypesDoNotUnify] src/CoreFn/Expr.purs:117:8 117 derive instance functorBind :: Functor Bind ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Could not match type b1 with type a2 while trying to match type Expr b1 with type Expr t0 while checking that expression (map (map $f146)) $v148 has type Tuple (Tuple t0 Ident) (Expr t0) in value declaration functorBind where b1 is a rigid type variable a2 is a rigid type variable t0 is an unknown type

https://travis-ci.org/paulyoung/purescript-corefn/jobs/345111990#L1549

I guess the issue might just be that Functor can’t be derived, all though I’m not sure if there would be a more specific error if that was the case.

paulyoung · 2018-02-26T04:49:17Z

src/CoreFn/Expr.purs


 derive instance eqBind :: Eq a => Eq (Bind a)
 derive instance ordBind :: Ord a => Ord (Bind a)

+instance functorBindRec :: Functor Bind where
+  map f (NonRec a i e) = NonRec (f a) i (map f e)
+  map f (Rec ts) = Rec $ map (bimap (lmap f) (map f)) ts


@LiamGoodacre does this look right to you?

FWIW, stack build --ghc-options "-ddump-deriv" produced .stack-work/dist/x86_64-osx/Cabal-2.0.0.2/build/src/Language/PureScript/CoreFn/Expr.dump-deriv which contained this:

instance GHC.Base.Functor Language.PureScript.CoreFn.Expr.Bind where GHC.Base.fmap f_amsj (Language.PureScript.CoreFn.Expr.NonRec a1_amsk a2_amsl a3_amsm) (Language.PureScript.CoreFn.Expr.NonRec a1_amsk a2_amsl a3_amsm) = Language.PureScript.CoreFn.Expr.NonRec (f_amsj a1_amsk) ((\ b1_amsn -> b1_amsn) a2_amsl) (GHC.Base.fmap f_amsj a3_amsm) GHC.Base.fmap f_amso (Language.PureScript.CoreFn.Expr.Rec a1_amsp) = Language.PureScript.CoreFn.Expr.Rec (GHC.Base.fmap (\ b3_amsq -> case b3_amsq of { ((,) a1_amsr a2_amss) -> (,) ((\ b2_amst -> case b2_amst of { ((,) a1_amsu a2_amsv) -> (,) (f_amso a1_amsu) ((\ b1_amsw -> b1_amsw) a2_amsv) }) a1_amsr) (GHC.Base.fmap f_amso a2_amss) }) a1_amsp)

Upon closer inspection I'm pretty confident that this is equivalent.

paulyoung · 2018-03-01T06:33:17Z

src/CoreFn/Ident.purs

+  -- |
+  -- A generated name used only for type-checking
+  --
+  | UnusedIdent


I'm not sure whether to keep this or not. There's nothing that can be code-generated from it, but the case needs to be handled or unsafePartial used.

paulyoung · 2018-03-01T06:43:48Z

I'm seeing 2 issues with converting corefn.json files that are produced with pulp init.

I'm looking into whether this is an issue with my conversion or the original Haskell version.

paulyoung · 2018-03-01T06:47:25Z

Actually, everything is fine. Some modules in the output directory were old and using the old format.

paulyoung · 2018-03-03T08:49:05Z

src/CoreFn/FromJSON.purs

+      >>= readArray
+      >>= traverse identFromJSON
+
+    moduleDecls <- pure []


paulyoung · 2018-03-03T19:44:16Z

src/CoreFn/FromJSON.purs

+  type_ <- readProp "metaType" json >>= readString
+  case type_ of
+    "IsConstructor" -> isConstructorFromJSON json
+    "IsNewType" -> pure IsNewtype


The test failing here might be because of the casing. Probably needs to be "IsNewtype".

paulyoung added 19 commits February 4, 2018 22:56

[WIP]

f704970

[WIP]

edd9f49

[WIP]

fa468c1

Comments

f9d124d

Replace inline check with object function

5667ca6

Better errors for comments

266d666

[WIP] Bindings

065a58d

[WIP]

b0f8395

Fix Meta

747a030

[WIP]

ef418d6

Literals

79ebddc

Fix expr binding

74c9937

[WIP]

b63072d

[WIP]

6edfba7

[WIP]

fe8c9f8

Fix Show instance

0c98ff5

Case expressions, case alternatives, and binders

312e052

Let

4700cda

General cleanup, remove Generic

fc4a0e2

paulyoung self-assigned this Feb 15, 2018

paulyoung mentioned this pull request Feb 15, 2018

Determining the location of FFI files by module name purescript/purescript#3162

Closed

paulyoung commented Feb 15, 2018

View reviewed changes

paulyoung added 3 commits February 21, 2018 21:26

Update CaseAlternative to take a record

4e437e8

Make Module a newtype

ec6fe80

Port tests

dc9ec66

Update attempt at a Functor instance

aa1d736

paulyoung commented Feb 22, 2018

View reviewed changes

paulyoung changed the title ~~[WIP] Support new JSON format~~ Support new JSON format Feb 22, 2018

paulyoung mentioned this pull request Feb 25, 2018

Uncomment broken Functor instances #43

Closed

Fix NonRec Bind constructor, add functor instances

69407c7

paulyoung commented Feb 26, 2018

View reviewed changes

Only export moduleFromJSON

1646939

This was referenced Feb 26, 2018

Add Functor, Foldable, Traversable instances #23

Closed

Monomorphic Expr #37

Closed

Case expressions #38

Closed

paulyoung commented Mar 1, 2018

View reviewed changes

paulyoung commented Mar 3, 2018

View reviewed changes

src/CoreFn/FromJSON.purs Outdated

>>= readArray

>>= traverse identFromJSON

moduleDecls <- pure []

Copy link

Owner Author

paulyoung Mar 3, 2018

Choose a reason for hiding this comment

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

Whoops

Fix decls

1b9fc99

paulyoung commented Mar 3, 2018

View reviewed changes

paulyoung added 2 commits March 3, 2018 17:04

Fix IsNewtype

4b5c4c8

Fix recordFromJSON

8f3a0e5

paulyoung merged commit bec59ca into master Apr 15, 2018

paulyoung deleted the paulyoung/new-format branch April 15, 2018 22:15

This was referenced Apr 27, 2018

Ready to publish? #44

Open

Distinguish between Ident and GenIdent? #26

Closed

paulyoung mentioned this pull request Jun 11, 2020

Fully generalization Module Ann to Module a #62

Open

Support new JSON format #42

Support new JSON format #42

Uh oh!

Conversation

paulyoung commented Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabejohnson Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulyoung commented Feb 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulyoung commented Mar 1, 2018

Uh oh!

paulyoung commented Mar 1, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulyoung commented Feb 15, 2018 •

edited

Loading

gabejohnson Feb 15, 2018 •

edited

Loading