[wip] Add support for lists #39

Warry · 2019-07-08T18:07:38Z

fixes #27

src/compiler/AST/Frontend.elm

harrysarson · 2019-07-08T18:28:29Z

src/compiler/Stage/Emit/JavaScript.elm

+        List { list } ->
+            "["
+            ++ (List.map emitExpr list |> String.join ", ")
+            ++ "]"


Official elm represents lists as linked lists rather than javascript arrays which makes concatenation work. I suspect that elm-in-elm should follow the same approach

Interestingly official Elm keeps list literals as Lists of the language it's defined in (see AST.Source, AST.Canonical, AST.Optimized) and when emitting JS (Generate.JavaScript.Expression) either returns Nil or calls fromArray and generates a JS list as an argument for it.

We'll definitely have to plug into the elm/core List definition (btw funny thing, it's JS-only 😭 ), but we can do that later. What @Warry has right now is not that far off, and for now good enough I think. When we have enough stuff working that it stops being consistent with each other, we'll fix it.

WDYT @harrysarson @Warry ?

That is interesting, also interesting why the list definition is in javascript.

Regarding long term plan sounds good - the point at which it stops working will probably be when we implement (::).

harrysarson · 2019-07-08T18:28:48Z

Looks nice!

Warry · 2019-07-08T19:03:08Z

To clarify: I toyed around to get to understand what I'm touching. The given assignment doesn't cover parsing, so it'll be deleted.
My plan this evening:

Seek what errors the elm compiler references
Write code that cover more cases, including breaking
Adding tests for the Type Inference stage

src/compiler/AST/Common/Type.elm

Janiczek · 2019-07-08T20:08:42Z

@Warry if you want to do everything in one go (ie. with parsers included), feel free to; I've just separated the issues as I think that would be more helpful (smaller problems) for newcomers. If you want to do parsers with the rest of the stuff, I won't mind.

src/compiler/Error.elm

Janiczek · 2019-07-09T20:34:13Z

src/compiler/Stage/InferTypes.elm

+                Type.List type1 ->
+                    List.foldl
+                        (\(entry, type2) acc ->
+                            (equals type1 type2) :: acc


I'd just change the names a bit:

type1 is listParamType

entry we don't care about ➡️ _

type2 can be listElementType

and acc is equations or eqs?

Anyway that's how I'd write it. 😅

tests/EmitTest.elm

Janiczek · 2019-07-09T21:22:15Z

tests/InferTypesTest.elm

+                                ++ Type.toString t2
+                                ++ " don't match."
+                        _ ->
+                            "wft"


probably just use Error.toString? 🙈

yes! The state of that PR is a mess, I won't leave it that way very long ^^ I guess I just have to Error.toString Error.TypeError typeError.

tests/ParserTest.elm

Janiczek · 2019-07-11T11:01:36Z

tests/InferTypesTest.elm

+                  )
+                , ( "more items types"
+                  , Canonical.List [ Canonical.Literal (Int 2), Canonical.Literal (Int 3), Canonical.Literal (Bool False) ]
+                  , Err (Error.TypeMismatch (Type.Int) (Type.Bool))


some extra parentheses, I expect running elm-format would solve those

tests/InferTypesTest.elm

Janiczek · 2019-07-11T11:12:44Z

tests/EmitTest.elm

+                      )
+                    , ( "simple list2"
+                      , List [ typed (Literal (String "one")), typed (Literal (Int 2)), typed (Literal (Int 3)) ]
+                      , "[\"one\", 2, 3]"


I'd drop this test as that source Expr is a bit nonsensical. Or let's change it to a list of three strings?

Janiczek · 2019-07-11T11:13:34Z

src/compiler/Stage/Parse/Parser.elm

+            , item = PP.subExpression 0 config
+            , trailing = P.Forbidden
+            }
+        |> P.inContext InList


Cool to see P.sequence works well. I'm a bit careful regarding those helpers 😅

It's not perfect, but I guess it was specifically design with that usecase in mind, so I'm confident it can stay like this until perfs become a thing or at least until (::).

src/compiler/Error.elm

src/compiler/Stage/Desugar.elm

Janiczek · 2019-07-11T11:20:02Z

src/compiler/Stage/Emit/JavaScript.elm

@@ -66,6 +66,11 @@ emitExpr ( expr, _ ) =
            in
            "((() => {" ++ bindingsJS ++ "; return " ++ emitExpr body ++ ";})())"

+        List list_ ->


again list_ -> items or list 😄

Janiczek · 2019-07-11T11:20:51Z

src/compiler/Stage/InferTypes/AssignIds.elm

@@ -170,3 +170,21 @@ assignIdsWith idSource expr =

        Canonical.Unit ->
            wrap idSource Typed.Unit
+
+        Canonical.List list_ ->


aand again list_ -> items or list. Seeing as the rest of the function uses items_, I guess items makes most sense here.

…n the list

Janiczek · 2019-07-11T11:58:25Z

src/compiler/Stage/Desugar.elm

+desugarList : (Frontend.Expr -> Result DesugarError Canonical.Expr) -> List Frontend.Expr -> Result DesugarError Canonical.Expr
+desugarList recurse list =
+    List.map recurse list
+        |> List.foldl (Result.map2 (::)) (Ok [])


some Debug.logs:

source: [ 1, 50, 999 ] after parsing: List [Literal (Int 1),Literal (Int 50),Literal (Int 999)] after desugaring: List [Literal (Int 999),Literal (Int 50),Literal (Int 1)] after typing: (List [(Literal (Int 999),Int),(Literal (Int 50),Int),(Literal (Int 1),Int)],Var 4) after emit: [999, 50, 1]

I believe the foldr was correct. The comment below was just a question of why is the foldr there, didn't want to indicate that it's wrong ... :) sorry for confusion!

Janiczek

🎉

Warry added 2 commits July 8, 2019 20:01

Add support for lists

b7bebe7

fixes #27

add dummy type inference

18ee49d

harrysarson reviewed Jul 8, 2019

View reviewed changes

Janiczek reviewed Jul 8, 2019

View reviewed changes

src/compiler/AST/Common/Type.elm Outdated Show resolved Hide resolved

Warry and others added 2 commits July 9, 2019 21:47

complete rework

afcbbf5

Delete package-lock.json

47f5329

Janiczek reviewed Jul 9, 2019

View reviewed changes

src/compiler/Error.elm Outdated Show resolved Hide resolved

Janiczek reviewed Jul 9, 2019

View reviewed changes

tests/EmitTest.elm Show resolved Hide resolved

Warry added 2 commits July 9, 2019 22:47

add tests inference file

2868406

Merge branch 'add-lists' of github.com:Warry/compiler into add-lists

530bc6b

Janiczek reviewed Jul 9, 2019

View reviewed changes

Warry and others added 9 commits July 9, 2019 23:45

debug unify

515c317

iterrating over lunch

8660e45

Merge, do ID assigning and equation generation

481f0db

Merge branch 'master' into add-lists

1cb9a03

Format

b716e4a

Make it compile and pass tests

489a86b

wording

e95eec3

finalizing

cd60c99

add comment for future

87ca12e

Janiczek reviewed Jul 11, 2019

View reviewed changes

tests/ParserTest.elm Outdated Show resolved Hide resolved

Janiczek reviewed Jul 11, 2019

View reviewed changes

run elm-format

9ebe5bf

Janiczek reviewed Jul 11, 2019

View reviewed changes

tests/InferTypesTest.elm Show resolved Hide resolved

Janiczek reviewed Jul 11, 2019

View reviewed changes

tests/InferTypesTest.elm Outdated Show resolved Hide resolved

Janiczek reviewed Jul 11, 2019

View reviewed changes

src/compiler/Error.elm Outdated Show resolved Hide resolved

Janiczek reviewed Jul 11, 2019

View reviewed changes

src/compiler/Error.elm Outdated Show resolved Hide resolved

Janiczek reviewed Jul 11, 2019

View reviewed changes

src/compiler/Stage/Desugar.elm Outdated Show resolved Hide resolved

Janiczek reviewed Jul 11, 2019

View reviewed changes

src/compiler/Stage/Desugar.elm Show resolved Hide resolved

Janiczek reviewed Jul 11, 2019

View reviewed changes

Warry added 5 commits July 11, 2019 13:29

improve tests

cfb7e79

remove unused code, ensure orders of types in errors is the same as i…

09ce4be

…n the list

remove useless full qualification

eb056fa

remove silly debug test

eefc30e

remove one last list_

33fc387

Janiczek reviewed Jul 11, 2019

View reviewed changes

Warry and others added 7 commits July 11, 2019 14:01

remove last list_

613e9a8

fix list order in desugar

b7a8efe

Better tests

1a86301

Fix substitution

5e21c70

Remove Debug.logs

cfae5ef

elm-analyse

e85fdf0

Make naming consistent

f91f21d

Janiczek approved these changes Jul 11, 2019

View reviewed changes

Janiczek merged commit c6ffb45 into elm-in-elm:master Jul 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wip] Add support for lists #39

[wip] Add support for lists #39

Warry commented Jul 8, 2019

harrysarson Jul 8, 2019

Janiczek Jul 8, 2019

harrysarson Jul 8, 2019

harrysarson commented Jul 8, 2019

Warry commented Jul 8, 2019

Janiczek commented Jul 8, 2019

Janiczek Jul 9, 2019

Janiczek Jul 9, 2019

Warry Jul 10, 2019

Janiczek Jul 11, 2019

Janiczek Jul 11, 2019

Janiczek Jul 11, 2019

Warry Jul 11, 2019 •

edited

Loading

Janiczek Jul 11, 2019

Janiczek Jul 11, 2019

Janiczek Jul 11, 2019

Janiczek left a comment

[wip] Add support for lists #39

[wip] Add support for lists #39

Conversation

Warry commented Jul 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harrysarson commented Jul 8, 2019

Warry commented Jul 8, 2019

Janiczek commented Jul 8, 2019

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

Warry Jul 11, 2019 • edited Loading

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

Janiczek left a comment

Choose a reason for hiding this comment

Warry Jul 11, 2019 •

edited

Loading