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

Add erlang-module support (update) #2806

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 80 additions & 10 deletions lib/livebook/runtime/evaluator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,9 @@ defmodule Livebook.Runtime.Evaluator do
end

start_time = System.monotonic_time()

{eval_result, code_markers} = eval(language, code, context.binding, context.env)

evaluation_time_ms = time_diff_ms(start_time)

%{tracer_info: tracer_info} = Evaluator.IOProxy.after_evaluation(state.io_proxy)
Expand Down Expand Up @@ -624,6 +626,11 @@ defmodule Livebook.Runtime.Evaluator do
|> Map.update!(:context_modules, &(&1 ++ prev_env.context_modules))
end

# TODO: Make sure to change this into a folder which pertains the session.
defp tmp_filename() do
:lists.flatten(:io_lib.format("/tmp/erlang-eval-~p.tmp", [:erlang.phash2(make_ref())]))
end
Copy link
Member

Choose a reason for hiding this comment

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

We get tmp_dir on init, so we could use that as the base. But let's try the fileless approach first.

Copy link
Contributor

Choose a reason for hiding this comment

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

The fileless approach won't work.

@fnchooft we pass a tmp_dir to init:

tmp_dir = Keyword.get(opts, :tmp_dir)

Can you store it as part of the state and use it to build the tmp directory? Notice that it may be nil in case of read-only file systems, so we should return an error in those cases saying you can't define erlang modules in read-only file systems. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do - some subfolder was not created ( added a File.mkdir_p )


defp eval(:elixir, code, binding, env) do
{{result, extra_diagnostics}, diagnostics} =
Code.with_diagnostics([log: true], fn ->
Expand Down Expand Up @@ -688,20 +695,87 @@ defmodule Livebook.Runtime.Evaluator do
{result, code_markers}
end

# Erlang code is either statements as currently supported, or modules.
# In case we want to support modules - it makes sense to allow users to use
# includes, defines and thus we use the epp-module first - try to find out
# if we have a module attribute, and if so deem it a module.
# If no module attribute was found the previous code is called.
defp eval(:erlang, code, binding, env) do
filename = tmp_filename()
:file.write_file(filename, code)

eval_result =
case :epp.parse_file(filename, [], []) do
{:ok, forms} ->
case :lists.keyfind(:module, 3, forms) do
{:attribute, _lineno, :module, module} ->
eval_erlang_module(code, module, forms, binding, env)

false ->
case :erl_scan.string(String.to_charlist(code), {1, 1}, [:text]) do
{:ok, tokens, _} ->
eval_erlang_statements(code, tokens, binding, env)

{:error, {location, module, description}, _end_loc} ->
process_erlang_error(env, code, location, module, description)
end
end

{:error, epp_parse_errors} ->
{{:error, :error, epp_parse_errors, []}, []}
end

# Clean up after ourselves.
:file.delete(filename)
Comment on lines +781 to +782
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Clean up after ourselves.
:file.delete(filename)
_ = File.rm(filename)

eval_result
end

# Create module - tokens from string
# Based on: https://stackoverflow.com/questions/2160660/how-to-compile-erlang-code-loaded-into-a-string
# Step 1: Scan the code using the epp:parse_file erlang primitive
# - epp will do macro expansion etc.
# Step 2: Convert to forms
# Step 3: Extract module name
# Step 4: Compile and load
# Step 5: If compile success - register module

defp eval_erlang_module(_code, module, forms, binding, env) do
try do
case :compile.forms(forms) do
Copy link
Contributor

Choose a reason for hiding this comment

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

you can pass the options return_errors and return_warnings, so it returns errors and warnings and convert them to diagnostics. But I think we can do it in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a case - later we should clean it up as you indicated.

{:ok, _, binary_module} ->
{:module, module} = :code.load_binary(module, ~c"#{module}.beam", binary_module)
Copy link
Member

Choose a reason for hiding this comment

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

Technically :code.load_binary doesn't use the filename, but if we want to be accurate, we can do this:

Suggested change
{:module, module} = :code.load_binary(module, ~c"#{module}.beam", binary_module)
file =
if ebin_path = ebin_path() do
Path.join(ebin_path, "#{module}.beam")
else
"#{module}.beam"
end
{:module, module} = :code.load_binary(module, String.to_charlist(file), binary_module)


# Registration of module
env = %{env | module: module, versioned_vars: %{}}
Evaluator.Tracer.trace({:on_module, binary_module, %{}}, env)
result = {:ok, module}
{{:ok, result, binding, env}, []}

:error ->
# TODO: Return errors and warnings and convert them to diagnostics
{{:error, :error, "compilation failed", []}, []}
end
catch
fnchooft marked this conversation as resolved.
Show resolved Hide resolved
kind, error ->
stacktrace = prune_stacktrace(:erl_eval, __STACKTRACE__)
{{:error, kind, error, stacktrace}, []}
end
end

defp eval_erlang_statements(code, tokens, binding, env) do
try do
erl_binding =
Enum.reduce(binding, %{}, fn {name, value}, erl_binding ->
:erl_eval.add_binding(elixir_to_erlang_var(name), value, erl_binding)
end)

with {:ok, tokens, _} <- :erl_scan.string(String.to_charlist(code), {1, 1}, [:text]),
{:ok, parsed} <- :erl_parse.parse_exprs(tokens),
with {:ok, parsed} <- :erl_parse.parse_exprs(tokens),
{:value, result, new_erl_binding} <- :erl_eval.exprs(parsed, erl_binding) do
# 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},
Expand Down Expand Up @@ -731,10 +805,6 @@ defmodule Livebook.Runtime.Evaluator do

{{:ok, result, binding, env}, []}
else
# Tokenizer error
{:error, {location, module, description}, _end_loc} ->
process_erlang_error(env, code, location, module, description)

# Parser error
{:error, {location, module, description}} ->
process_erlang_error(env, code, location, module, description)
Expand Down Expand Up @@ -873,11 +943,11 @@ defmodule Livebook.Runtime.Evaluator do
do: {:module, module},
into: identifiers_used

# Note: `module_info` works for both Erlang and Elixir modules, as opposed to `__info__`
identifiers_defined =
for {module, _line_vars} <- tracer_info.modules_defined,
version = module.__info__(:md5),
do: {{:module, module}, version},
into: identifiers_defined
for {module, _line_vars} <- tracer_info.modules_defined, into: identifiers_defined do
{{:module, module}, module.module_info(:md5)}
end

# Aliases

Expand Down
69 changes: 69 additions & 0 deletions test/livebook/runtime/evaluator_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,75 @@ defmodule Livebook.Runtime.EvaluatorTest do
assert_receive {:runtime_evaluation_response, :code_1, terminal_text("6"), metadata()}
end

test "evaluate erlang-module code", %{evaluator: evaluator} do
Evaluator.evaluate_code(
evaluator,
:erlang,
"-module(tryme). -export([go/0]). go() ->{ok,went}.",
:code_4,
[]
)

assert_receive {:runtime_evaluation_response, :code_4, terminal_text(_), metadata()}
end

test "evaluate erlang-module error function already defined", %{evaluator: evaluator} do
Evaluator.evaluate_code(
evaluator,
:erlang,
"-module(tryme). -export([go/0]). go() ->{ok,went}. go() ->{ok,went}.",
:code_4,
[]
)

assert_receive {
:runtime_evaluation_response,
:code_4,
error(message),
metadata()
}
fnchooft marked this conversation as resolved.
Show resolved Hide resolved

assert message =~ "compilation failed"
end

test "evaluate erlang-module error - expression after module", %{evaluator: evaluator} do
Evaluator.evaluate_code(
evaluator,
:erlang,
"-module(tryme). -export([go/0]). go() ->{ok,went}. go() ->{ok,went}. A = 1.",
:code_4,
[]
)

assert_receive {
:runtime_evaluation_response,
:code_4,
error(message),
metadata()
}

assert message =~ "compilation failed"
end

test "evaluate erlang-module error - two modules", %{evaluator: evaluator} do
Evaluator.evaluate_code(
evaluator,
:erlang,
"-module(one). -export([go/0]). go() ->{ok,one}. -module(two). -export([go/0]). go() ->{ok,two}.",
:code_4,
[]
)

assert_receive {
:runtime_evaluation_response,
:code_4,
error(message),
metadata()
}

assert message =~ "compilation failed"
end

test "mixed erlang/elixir bindings", %{evaluator: evaluator} do
Evaluator.evaluate_code(evaluator, :elixir, "x = 1", :code_1, [])
Evaluator.evaluate_code(evaluator, :erlang, "Y = X.", :code_2, [:code_1])
Expand Down
Loading