leostera / caramel

:candy: a functional language for building type-safe, scalable, and maintainable applications
https://caramel.run
Apache License 2.0
1.05k stars 25 forks source link

Nested let-in expressions compile to wrong erlang #68

Closed michallepicki closed 3 years ago

michallepicki commented 3 years ago

Describe the bug When a let expression value contains a let expression, the generated erlang is "flattened" and running the code produces an incorrect result (or may not compile at all)

To Reproduce

  1. Create a main.ml file containing
    
    let print_int thing = Io.format "~0tp~n" [ thing ]

let main _ = let one = 1 in let three = let two = one + 1 in two + 1 in print_int three

I would format it slightly differently, to have the last line nested one level less, or with parens around the nested let-in, but that's how the formatter prints it.

2. Run command 
```bash
$ caramel compile main.ml && escript main.erl
  1. See the incorrect result:
    Compiling main.erl      OK
    2

The generated erlang is:

% Source code generated with Caramel.
-module(main).

-export([main/1]).
-export([print_int/1]).

-spec print_int(_) -> ok.
print_int(Thing) -> io:format(<<"~0tp~n">>, [Thing | []]).

-spec main(_) -> ok.
main(_) ->
  One = 1,
  Three = Two = erlang:'+'(One, 1),
erlang:'+'(Two, 1),
  print_int(Three).

Expected behavior It should print 3, just like this Ocaml file would:

(* let print_int thing = Io.format "~0tp~n" [ thing ] *)

let main _ =
  let one = 1 in
  let three =
    let two = one + 1 in
      two + 1 in
    print_int three ;;

main ()
$ ocaml main.ml
3

The generated Erlang should probably be:

-module(main).

-export([main/1]).
-export([print_int/1]).

-spec print_int(_) -> ok.
print_int(Thing) -> io:format(<<"~0tp~n">>, [Thing | []]).

-spec main(_) -> ok.
main(_) ->
  One = 1,
  Three = begin
    Two = erlang:'+'(One, 1),
    erlang:'+'(Two, 1)
  end,
  print_int(Three).

Environment (please complete the following information):

shawn-mcginty commented 3 years ago

What should the erl code look like in this case? I'm not super familiar with erlang and I couldn't find a nice way to do nested expressions (after googling for only a few minutes). I was thinking wrapping in an anonymous function?

-module(main).

-export([main/1]).
-export([print_int/1]).

-spec print_int(_) -> ok.
print_int(Thing) -> io:format(<<"~0tp~n">>, [Thing | []]).

-spec main(_) -> ok.
main(_) ->
  One = 1,
  Three = fun() ->
    Two = erlang:'+'(One, 1),
    erlang:'+'(Two, 1)
  end().
  print_int(Three).

This looks ugly to me though.

michallepicki commented 3 years ago

edit: I added this to the issue description.

Hey @shawn-mcginty , I think it would be a begin / end expression block:

-module(main).

-export([main/1]).
-export([print_int/1]).

-spec print_int(_) -> ok.
print_int(Thing) -> io:format(<<"~0tp~n">>, [Thing | []]).

-spec main(_) -> ok.
main(_) ->
  One = 1,
  Three = begin
    Two = erlang:'+'(One, 1),
    erlang:'+'(Two, 1)
  end,
  print_int(Three).
shawn-mcginty commented 3 years ago

Exactly what I was looking for, thank you @michallepicki !

michallepicki commented 3 years ago

There is a related ticket to translate the ocaml expression sequences to that as well #59 (~don't worry about the && / andalso ementioned there, there is a separate ticket to discuss how those should behave, I should change the example there to have something different.~ edit: it was updated)

erszcz commented 3 years ago

@michallepicki I think translating an ML-style let .. in to an Erlang begin .. end it too simplistic. Please consider the following example (also using the sequence operator ;):

-module(vars_leak_out_of_begin_end).

-compile(export_all).

%% Leaks `A` to the outer scope.
test1() ->
    begin
        A = 1
    end,
    A.

%% Does not compile, but it's a Good Thing!
%test2() ->
%    (fun () ->
%             A = 1
%     end)(),
%    A.

%% let a = 1 in a + 2
let_approach_1_example_1() ->
    begin
        A = 1,
        A + 2
    end.

%% (let a = 1 in print_endline (string_of_int a);
%%  let a = 5 in a + 2);;
%% Compiles with a warning, fails pattern matching at runtime!
%let_approach_1_example_2() ->
%    begin
%        A = 1,
%        io:format("~p\n", [A])
%    end,
%    begin
%        A = 5,
%        A + 2
%    end.

%% let a = 1 in a + 2
let_approach_2_example_1() ->
    A = 1,
    (fun () ->
             A + 2
     end)().

%% (let a = 1 in print_endline (string_of_int a);
%%  let a = 5 in a + 2);;
%% Compiles with a warning, fails pattern matching at runtime!
let_approach_2_example_2() ->
    A = 1,
    (fun () ->
             io:format("~p\n", [A])
     end)(),
    A = 5,
    (fun () ->
             A + 2
     end)().

%% let a = 1 in a + 2
let_approach_3_example_1() ->
    %% `_A` silences the "unused variable" warning on `erlc` level
    %% if A was not used in the fun body.
    %% This might be useful if `caramel compile` emits this warning on its own.
    (fun (_A) ->
             _A + 2
     end)(1).

%% (let a = 1 in print_endline (string_of_int a);
%%  let a = 5 in a + 2);;
let_approach_3_example_2() ->
    (fun (_A) ->
             io:format("~p\n", [_A])
     end)(1),
    (fun (_A) ->
             _A + 2
     end)(5).
michallepicki commented 3 years ago

Maybe adding unique suffixes (e.g. numbers) to resolve naming clashes would allow to avoid the IIFEs?

erszcz commented 3 years ago

@michallepicki Indeed, I think picking fresh variable names would work, if avoiding IIFEs is a goal. And it could be considered one, given IIFEs imply a slightly bigger overhead than a translation with plain variables.

michallepicki commented 3 years ago

@ostera thoughts? I don't think it would be hard to avoid IIFEs

leostera commented 3 years ago

I've referred to this in @erszcz's PR: https://github.com/AbstractMachinesLab/caramel/pull/81#issuecomment-799158500

michallepicki commented 3 years ago

I'll start working on it, and hopefully I will also make variables shadowing work as well, adding suffixes to names in a similar fashion

michallepicki commented 3 years ago

The example from issue description will be already fixed in #80 (I added it as a test). There will be still issues with variable names mentioned in https://github.com/AbstractMachinesLab/caramel/issues/68#issuecomment-795916884 , I'm looking into generating unique names similarily to how Gleam does

michallepicki commented 3 years ago

Since the original example is now working, let's close this and have a new issue for generating unique variable names to deal with Erlang scoping issues related to expression blocks