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.

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

IR Typing: IR Translation of concatMap is unsound, misses type application #368

Closed
frank-emrich opened this issue Jul 12, 2018 · 3 comments
Assignees
Labels
bug IR Typing Bugs found by the IR typechecker

Comments

@frank-emrich
Copy link
Contributor

frank-emrich commented Jul 12, 2018

The IR typechecker caught the following problem:

The IR version of concatMap from the Links prelude is as follows. Here, 234 is the variable bound to the function, which has the polymorphic type forall a::(Any,Any),b::Row,c.((a::Any) ~b~> [c], [a::Any]) ~b~> [c]

`Rec ([((234,
           (forall a::(Any,Any),b::Row,c.((a::Any) ~b~> [c], [a::Any]) ~b~> [c],
            "concatMap", `Global)),
          ([(655, (`Any, `Any), `Type (...));
             (662, (`Unl, `Any), `Row (...));
             (663, (`Unl, `Any), `Type (...))],
           [(232, ((_::Any) ~> [_], "", `Local));
             (233, ([_::Any], "", `Local))],
           ([`Let (((230, ([_::Any], "l", `Local)),
                    ([], `Return (`Variable (233)))));
              `Let (((231, ((_::Any) ~> [_], "f", `Local)),
                     ([], `Return (`Variable (232)))));
              `Let (((237, ([_::Any], "", `Local)),
                     ([], `Return (`Variable (230)))))
              ],
            `If ((`ApplyPure ((`TApp ((`Variable (13),
                                       [`Type (`MetaTypeVar (...));
                                         `Row (({}, ..., false))])),
                               [`Variable (237);
                                 `TApp ((`Variable (55),
                                         [`Type (`MetaTypeVar (...))]))
                                 ])),
                  ([],
                   `Return (`TApp ((`Variable (55),
                                    [`Type (`MetaTypeVar (...))])))),
                  ([`Let (((238, (_::Any, "", `Local)),
                           ([],
                            `Apply ((`TApp ((`Variable (58),
                                             [`Type (`MetaTypeVar (...));
                                               `Row (({}, ..., false))])),
                                     [`Variable (237)])))));
                     `Let (((239, ([_::Any], "", `Local)),
                            ([],
                             `Apply ((`TApp ((`Variable (59),
                                              [`Type (`MetaTypeVar (...));
                                                `Row (({}, ..., false))])),
                                      [`Variable (237)])))));
                     `Let (((236, ([_::Any], "tl", `Local)),
                            ([], `Return (`Variable (239)))));
                     `Let (((235, (_::Any, "hd", `Local)),
                            ([], `Return (`Variable (238)))));
                     `Let (((241, ([_], "", `Local)),
                            ([],
                             `Apply ((`Variable (231), [`Variable (235)])))));
                     `Let (((240, ([_], "", `Local)),
                            ([],
                             `Apply ((`Variable (234),
                                      [`Variable (231); `Variable (236)])))))
                     ],
                   `Return (`ApplyPure ((`TApp ((`Variable (57),
                                                 [`Type (`MetaTypeVar (...));
                                                   `Row (({}, ..., false))])),
                                         [`Variable (241); `Variable (240)])))))))),
None, `Unknown)]);

The erroneous part is

`Apply ((`Variable (234),
           [`Variable (231); `Variable (236)]))

denoting the recursive call in concatMap.
Since the function is polymorphic, there must be a type application happening beforehand.

@frank-emrich frank-emrich added this to To do in Typed IR via automation Jul 12, 2018
@jamescheney
Copy link
Contributor

Might be easier to localize the problem in typechecking/desugaring by deleting parts of concatMap until the error goes away. E.g. does

fun concatMap'(f, l) {
  switch (l) {
    case hd::tl -> concatMap'(f, tl)
  }
}

have the same problem? If so, what about

fun concatMap''(f, l) {
  concatMap''(f, tl)
}

(or indeed why doesn't any other recursive function have the same problem?)

@jamescheney jamescheney removed this from To do in Typed IR Jul 12, 2018
@jamescheney jamescheney added this to To do in Typed IR via automation Jul 12, 2018
@frank-emrich
Copy link
Contributor Author

frank-emrich commented Jul 13, 2018

Good call. Actually, even

fun foo(f) { foo(f) }

has the same problem. Howver, it's not the case that we never emit type applications. I'll investigate it further.

@jstolarek
Copy link
Contributor

Test added in 9a2880c. Closing.

Typed IR automation moved this from To do to Done Jul 29, 2019
@jamescheney jamescheney moved this from To do to Done in Type system cleanup Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug IR Typing Bugs found by the IR typechecker
Projects
Typed IR
  
Done
Development

No branches or pull requests

4 participants