Add erlang-module support #2094

fnchooft · 2023-07-19T18:27:00Z

The code is deemed a module definition if the first line start with "-module(".

In this case the entire code-block is interpreted as erlang-module and if there are no errors the module is compiled and loaded.

An example for a working livebook (livemd-file):

Untitled notebook

Erlang - on the fly ( inline_module ) via Livebook

-module(inline_module2).

-export([go/1]).

go(jose) -> {ok,<<"thanks Jose! :)">>};
go(fabian) -> {ok,<<"kkkk - this Elixir-code is heavily Erlang-flavoured heim??? :)">>};
go(_) -> {ok, <<"thanks contributers!">>}.

inline_module2:go(jose).

CLAassistant · 2023-07-19T18:27:05Z

All committers have signed the CLA.

josevalim · 2023-07-19T19:01:30Z

lib/livebook/runtime/evaluator.ex

+      Enum.map(
+        statements,
+        fn str ->
+          {:ok, result, _} = :erl_scan.string(String.to_charlist(str))


We need to handle errors accordingly. It will be similar to the eval_statements branch, there is probably a lot we can share between the two. :)

Thanks for the feedback - will adapt based on the original-code.

josevalim · 2023-07-19T19:02:07Z

lib/livebook/runtime/evaluator.ex

@@ -675,6 +675,47 @@ defmodule Livebook.Runtime.Evaluator do
  end

  defp eval(:erlang, code, binding, env) do
+    is_module = String.starts_with?(code, "-module(")


Instead of detecting based on a string, we can do the first pass with :erl_scan.string(String.to_charlist(str)) and then traverse all entries and see if any defines a module attribute.

Your opinion: Analyze the forms to only allow one module? Throw an error to instruct the user?
Seems more inline with how Erlang defines modules.

If you defined multiple modules we would have to have a lot more checks, and we would have to seperate the statements per module.

Yeah, for now, I think the best is to:

Check if it has attributes

If it has attributes, one of them (no more, no less) has to be a module attribute

josevalim · 2023-07-19T19:02:32Z

This is very exciting @fnchooft, I have added some comments. We need error handling and tests. :)

fnchooft · 2023-07-20T14:21:55Z

Hi Jose, still W.I.P...

Changes:

Basic try-catch for error-handling
Use the scanner for the whole code-block
Create forms from tokens - Erlang-code-conversion - I am not Elixir apt-yet.... :)
TODO: Refactor the common parts
TODO: Add tests

josevalim · 2023-07-20T16:49:11Z

lib/livebook/runtime/evaluator.ex

+        Enum.map(
+          form_statements,
+          fn {form_statement} ->
+            {:ok, form} = :erl_parse.parse_form(form_statement)


Instead of try/catch below, we should pattern match and handle those forms similar to the other branch. You can look into Elixir's with (which is similar to Erlang's maybe).

This has been cleaned up in the manner you proposed.

josevalim · 2023-07-20T16:49:31Z

lib/livebook/runtime/evaluator.ex

+    catch
+      kind, error ->
+        stacktrace = prune_stacktrace(:erl_eval, __STACKTRACE__)
+        {{:error, kind, error, stacktrace}, []}


I think this outer try-catch is not necessary. :)

Morning - this has been cleaned up in latest update.

jonatanklosko · 2023-07-20T16:52:26Z

Also note that we need to recognise which modules are defined and used by each evaluation and return this information, similarly to used_vars:

livebook/lib/livebook/runtime/evaluator.ex

Lines 766 to 774 in 9296c48

    
           # Simple heuristic to detect the used variables. We look at 
        
           # the tokens and assume all var tokens are used variables. 
        
           # This will not handle shadowing of variables in fun definitions 
        
           # and will only work well enough for expressions, not for modules. 
        
           used_vars = 
        
             for {:var, _anno, name} <- tokens, 
        
                 do: {erlang_to_elixir_var(name), nil}, 
        
                 into: MapSet.new(), 
        
                 uniq: true

For Elixir we use tracer, but as a reference that's how the entries look like:

livebook/lib/livebook/runtime/evaluator.ex

Lines 938 to 949 in 9296c48

    
           # Modules 
        
           identifiers_used = 
        
             for module <- tracer_info.modules_used, 
        
                 do: {:module, module}, 
        
                 into: identifiers_used 
        
           identifiers_defined = 
        
             for {module, _vars} <- tracer_info.modules_defined, 
        
                 version = module.__info__(:md5), 
        
                 do: {{:module, module}, version}, 
        
                 into: identifiers_defined

fnchooft · 2023-07-20T18:58:38Z

@jonatanklosko - Thanks for the insight! Question: In the case we would limit the Code-erlang-block to the definition of only one Erlang-module, not allowing other expressions for instance, would it be enough to only add the module_name to the used_identifiers?

jonatanklosko · 2023-07-21T10:53:14Z

If we enforce just a single module we can compute md5 of the source as the module version or phash of the forms, because we would know all the code is about the module.

josevalim

Looking good! Can you please add some tests that exercise the error branches too? Some that make the tokenizer fail after a module is defined. And another that makes the parser file :) thanks!

The code is deemed a module definition if the first line start with "-module(". In this case the entire code-block is interpreted as erlang-module and if there are no errors the module is compiled and loaded. Basic error handling has been added Basic tests have been added

fnchooft · 2023-07-24T19:21:29Z

Afternoon - had some time to add:

More tests ( expression after module / test for duplicate function definitions etc )

@jonatanklosko - I tried adding only the modulename + md5 to the logic - as you suggested.
However it still is not 100% ok.

For instance - it is not detecting correctly if a call to a function in the module is stale or not.
If the function is not defined the error-handling is ok, however it would be nice to get the "stale"-indication on the erlang-code-cell.

jonatanklosko · 2023-07-25T15:29:41Z

For instance - it is not detecting correctly if a call to a function in the module is stale or not.

Yeah, and the same is going to be the case if you define a module with go/0, have another cell call go()., and then change the module implementation. We need to analyse the AST to find function calls and from that get a list of modules used by the given evaluation (cell).

For elixir we use compiler tracer to get the referenced modules and we then we use this information here:

livebook/lib/livebook/runtime/evaluator.ex

Lines 861 to 864 in efeebe9

    
           identifiers_used = 
        
             for module <- tracer_info.modules_used, 
        
                 do: {:module, module}, 
        
                 into: identifiers_used

@josevalim I'm thinking that ideally we would emit the tracer events programmatically (defining a module, referencing functions) and then the rest is handled using the same code path. Does that make sense, or is there any reason not to do that?

josevalim · 2023-07-25T16:48:41Z

@jonatanklosko emulating the tracer or the tracer_info is perfectly fine for me. The only concern is that there is no env.

jonatanklosko · 2023-07-25T17:10:26Z

Ah true, actually it should be simpler to use %Evaluator.Tracer{} together with Evaluator.Tracer.apply_updates/2 directly. If there are issues, we can have something more separate.

Other notes: we need to persist the module bytecode with Evaluator.write_module!(module, bytecode) and also add it to env.context_modules.

fnchooft · 2024-02-16T13:21:25Z

@jonatanklosko - Apologies for letting this simmer for some months.

I have not been following changes in livebook - but is there still value in this approach? If I have some time I will rebase and re-read the comments.

Kind regards,
Fabian

jonatanklosko · 2024-02-16T15:56:30Z

@fnchooft no worries, nothing changed in this matter, so sounds good :)

josevalim reviewed Jul 19, 2023

View reviewed changes

fnchooft force-pushed the erlang_module_support branch from 3af8aa1 to 9296c48 Compare July 20, 2023 14:17

josevalim reviewed Jul 20, 2023

View reviewed changes

fnchooft force-pushed the erlang_module_support branch from 9296c48 to 1447859 Compare July 21, 2023 11:26

josevalim reviewed Jul 21, 2023

View reviewed changes

fnchooft force-pushed the erlang_module_support branch from 1447859 to 13e17f6 Compare July 24, 2023 19:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add erlang-module support #2094

Add erlang-module support #2094

fnchooft commented Jul 19, 2023 •

edited

CLAassistant commented Jul 19, 2023 •

edited

josevalim Jul 19, 2023

fnchooft Jul 19, 2023

josevalim Jul 19, 2023

fnchooft Jul 19, 2023

josevalim Jul 19, 2023

josevalim commented Jul 19, 2023

fnchooft commented Jul 20, 2023

josevalim Jul 20, 2023

fnchooft Jul 21, 2023

josevalim Jul 20, 2023

fnchooft Jul 21, 2023

jonatanklosko commented Jul 20, 2023 •

edited

fnchooft commented Jul 20, 2023

jonatanklosko commented Jul 21, 2023

josevalim left a comment

fnchooft commented Jul 24, 2023

jonatanklosko commented Jul 25, 2023 •

edited

josevalim commented Jul 25, 2023

jonatanklosko commented Jul 25, 2023

fnchooft commented Feb 16, 2024

jonatanklosko commented Feb 16, 2024

Add erlang-module support #2094

Are you sure you want to change the base?

Add erlang-module support #2094

Conversation

fnchooft commented Jul 19, 2023 • edited

Untitled notebook

Erlang - on the fly ( inline_module ) via Livebook

CLAassistant commented Jul 19, 2023 • edited

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

josevalim commented Jul 19, 2023

fnchooft commented Jul 20, 2023

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

jonatanklosko commented Jul 20, 2023 • edited

fnchooft commented Jul 20, 2023

jonatanklosko commented Jul 21, 2023

josevalim left a comment

Choose a reason for hiding this comment

fnchooft commented Jul 24, 2023

jonatanklosko commented Jul 25, 2023 • edited

josevalim commented Jul 25, 2023

jonatanklosko commented Jul 25, 2023

fnchooft commented Feb 16, 2024

jonatanklosko commented Feb 16, 2024

fnchooft commented Jul 19, 2023 •

edited

CLAassistant commented Jul 19, 2023 •

edited

jonatanklosko commented Jul 20, 2023 •

edited

jonatanklosko commented Jul 25, 2023 •

edited