eirproject / eir

Erlang ecosystem common IR
Apache License 2.0
250 stars 8 forks source link

Handle `file` directive correctly #18

Open hansihe opened 5 years ago

hansihe commented 5 years ago

The 'file' compiler directive is often used in generated code to specify a location in an origin file where the code was generated from. This needs to interact properly with diagnostics.

I'm thinking the best way of doing things would be to show compiler errors in the actual file, but display runtime errors in the origin file.

Unless you already had an implementation in mind, I think a decent way of doing things would be to store source span => origin file line mappings in an auxiliary data structure outside of the diagnostics itself, and reference that when generating debug information for the runtime.

Thoughts @bitwalker?

I implemented a stub implementation in preprocessor.rs in order to get code compiling.

hansihe commented 5 years ago

Code isn't pushed just yet

KronicDeth commented 5 years ago

If the -file directive maps directly to the file used in the Line chunk for the BEAM byte code line opcode, then the -file needs to be used at runtime also. It is how EEx templates in Phoenix report the error coming from the *.html.eex file instead of the View module's file even though the EEx generated function is put into the View module.

hansihe commented 5 years ago

Yeah, that sounds about right. That is more or less what I mean when I say it has to be referenced when generating debug info for the runtime

KronicDeth commented 5 years ago

Here's an example of the PageView module for a template Phoenix project.

View Module

defmodule EExTestWeb.PageView do
  use EExTestWeb, :view
end

EEx Template

```

<%= gettext "Welcome to %{name}!", name: "Phoenix" %>

A productive web framework that
does not compromise speed and maintainability.

```

Expanded Byte Code

``` label(1) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 1) func_info(module: EExTestWeb.PageView, function: :__info__, arity: 1) label(2) is_atom(fail_label: 1, argument: x(0)) select_val(argument: x(0), fail_label: 1, value_to_label: list(:compile, label(3), :md5, label(3), :attributes, label(3), :functions, label(4), :module, label(5), :macros, label(6), :deprecated, label(6))) label(3) move(source: x(0), destination: x(1)) move(source: EExTestWeb.PageView, destination: x(0)) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 1) call_ext_only(&:erlang.get_module_info/2) label(4) move(source: [{:__phoenix_recompile__?, 0}, {:__resource__, 0}, {:__templates__, 0}, {:render, 1}, {:render, 2}, {:template_not_found, 2}], destination: x(0)) return() label(5) move(source: EExTestWeb.PageView, destination: x(0)) return() label(6) move(source: nil, destination: x(0)) return() label(7) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 2) func_info(module: EExTestWeb.PageView, function: :__phoenix_recompile__?, arity: 0) label(8) save_cp() move(source: "*", destination: x(1)) move(source: "lib/eex_test_web/templates/page", destination: x(0)) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 2) call_ext(&Phoenix.Template.hash/2) bif2(fail_label: 0, import: &:erlang."=/="/2, argument1: <<74, 169, 168, 113, 242, 44, 183, 190, 236, 124, 123, 66, 24, 41, 18, 8>>, argument2: x(0), destination: x(0)) restore_cp() return() label(9) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) func_info(module: EExTestWeb.PageView, function: :__resource__, arity: 0) label(10) move(source: :page, destination: x(0)) return() label(11) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 2) func_info(module: EExTestWeb.PageView, function: :__templates__, arity: 0) label(12) move(source: {"lib/eex_test_web/templates/page", "*", ["index.html"]}, destination: x(0)) return() label(13) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) func_info(module: EExTestWeb.PageView, function: :render, arity: 1) label(14) move(source: %{}, destination: x(1)) call_only(&EExTestWeb.PageView.render/2) label(15) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) func_info(module: EExTestWeb.PageView, function: :render, arity: 2) label(16) is_atom(fail_label: 17, argument: x(0)) move(source: %{}, destination: x(2)) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) call_ext_only(&Phoenix.View.render/3) label(17) is_binary(fail_label: 18, argument: x(0)) jump(label: 19) label(18) allocate(words_of_stack: 0, live_x_register_count: 1) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) call_ext(&Kernel.inspect/1) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) gc_bif1(fail_label: 0, live_x_register_count: 1, import: &:erlang.byte_size/1, argument: x(0), destination: x(1)) bs_add(fail_label: 0, source1: x(1), source2: 47, unit: 1, destination: x(1)) bs_init2(fail_label: 0, size: x(1), words_of_stack: 0, live_x_register_count: 1, flags: 0, destination: x(1)) bs_put_string('render/2 expects template to be a string, got: ') bs_put_binary(fail_label: 0, size: :all, unit: 8, flags: 0, source: x(0)) move(source: x(1), destination: x(0)) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) call_ext(&ArgumentError.exception/1) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) call_ext(&:erlang.error/1) label(19) is_map(fail_label: 20, argument: x(1)) jump(label: 21) label(20) allocate(words_of_stack: 1, live_x_register_count: 2) move(source: x(0), destination: y(0)) move(source: x(1), destination: x(0)) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) call_ext(&Map.new/1) move(source: x(0), destination: x(1)) move(source: y(0), destination: x(0)) call_last(arity: 2, label: 16, deallocate_words_of_stack: 1) label(21) call_only(&EExTestWeb.PageView.render_template/2) label(22) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 2) func_info(module: EExTestWeb.PageView, function: :render_template, arity: 2) label(23) is_eq_exact(fail_label: 24, left: x(0), right: 6) move(source: x(1), destination: x(0)) call_only(&EExTestWeb.PageView."index.html"/1) label(24) is_map(fail_label: 27, argument: x(1)) get_map_elements(fail_label: 25, source: x(1), key_to_destination: list(:render_existing, x(2))) is_tagged_tuple(fail_label: 25, source: x(2), arity: 2, tag: EExTestWeb.PageView) get_tuple_element(source: x(2), element_number: 1, destination: x(4)) is_eq_exact(fail_label: 25, left: x(4), right: x(0)) move(source: nil, destination: x(0)) return() label(25) get_map_elements(fail_label: 26, source: x(1), key_to_destination: list(:template_not_found, x(2))) is_eq_exact(fail_label: 26, left: x(2), right: EExTestWeb.PageView) move(source: x(1), destination: x(2)) move(source: x(0), destination: x(1)) move(source: EExTestWeb.PageView, destination: x(0)) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 2) call_ext_only(&Phoenix.Template.raise_template_not_found/3) label(26) is_map(fail_label: 27, argument: x(1)) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 2) put_map_assoc(fail_label: 0, source: x(1), destination: x(1), live_x_register_count: 2, field_from_source: list(:template_not_found, EExTestWeb.PageView)) call_only(&EExTestWeb.PageView.template_not_found/2) label(27) test_heap(words_of_heap: 3, live_x_register_count: 2) put_tuple(size: 2, destination: x(0)) put(:badmap) put(x(1)) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 2) call_ext(&:erlang.error/1) label(28) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) func_info(module: EExTestWeb.PageView, function: :template_not_found, arity: 2) label(29) move(source: x(1), destination: x(2)) move(source: x(0), destination: x(1)) move(source: EExTestWeb.PageView, destination: x(0)) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 1) call_ext_only(&Phoenix.Template.raise_template_not_found/3) label(30) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 2) func_info(module: EExTestWeb.PageView, function: :"index.html", arity: 1) label(31) save_cp() move(source: "Welcome to %{name}!", destination: x(2)) move(source: "default", destination: x(1)) move(source: [{:name, "Phoenix"}], destination: x(3)) move(source: EExTestWeb.Gettext, destination: x(0)) line(4) call_ext(&Gettext.dgettext/4) is_tagged_tuple(fail_label: 32, source: x(0), arity: 2, tag: :safe) get_tuple_element(source: x(0), element_number: 1, destination: x(0)) jump(label: 34) label(32) is_binary(fail_label: 33, argument: x(0)) line(file_name: "lib/eex_test_web/templates/page/index.html.eex", line: 2) call_ext(&Plug.HTML.html_escape_to_iodata/1) jump(label: 34) label(33) line(4) call_ext(&Phoenix.HTML.Safe.to_iodata/1) label(34) test_heap(words_of_heap: 7, live_x_register_count: 1) put_list(head: x(0), tail: ["\n

A productive web framework that
does not compromise speed and maintainability.

\n
\n\n
\n
\n

Resources

\n \n
\n\n
\n

Help

\n \n
\n
\n"], destination: x(1)) put_list(head: "
\n

", tail: x(1), destination: x(1)) put_tuple(size: 2, destination: x(0)) put(:safe) put(x(1)) restore_cp() return() label(35) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 1) func_info(module: EExTestWeb.PageView, function: :module_info, arity: 0) label(36) move(source: EExTestWeb.PageView, destination: x(0)) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 1) call_ext_only(&:erlang.get_module_info/1) label(37) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 1) func_info(module: EExTestWeb.PageView, function: :module_info, arity: 1) label(38) move(source: x(0), destination: x(1)) move(source: EExTestWeb.PageView, destination: x(0)) line(file_name: "lib/eex_test_web/views/page_view.ex", line: 1) call_ext_only(&:erlang.get_module_info/2) ```

BEAM Bytecode without line expanded

``` label(1) line(0) func_info(module: EExTestWeb.PageView, function: :__info__, arity: 1) label(2) is_atom(fail_label: 1, argument: x(0)) select_val(argument: x(0), fail_label: 1, value_to_label: list(:compile, label(3), :md5, label(3), :attributes, label(3), :functions, label(4), :module, label(5), :macros, label(6), :deprecated, label(6))) label(3) move(source: x(0), destination: x(1)) move(source: EExTestWeb.PageView, destination: x(0)) line(0) call_ext_only(&:erlang.get_module_info/2) label(4) move(source: [{:__phoenix_recompile__?, 0}, {:__resource__, 0}, {:__templates__, 0}, {:render, 1}, {:render, 2}, {:template_not_found, 2}], destination: x(0)) return() label(5) move(source: EExTestWeb.PageView, destination: x(0)) return() label(6) move(source: nil, destination: x(0)) return() label(7) line(1) func_info(module: EExTestWeb.PageView, function: :__phoenix_recompile__?, arity: 0) label(8) save_cp() move(source: "*", destination: x(1)) move(source: "lib/eex_test_web/templates/page", destination: x(0)) line(1) call_ext(&Phoenix.Template.hash/2) bif2(fail_label: 0, import: &:erlang."=/="/2, argument1: <<74, 169, 168, 113, 242, 44, 183, 190, 236, 124, 123, 66, 24, 41, 18, 8>>, argument2: x(0), destination: x(0)) restore_cp() return() label(9) line(2) func_info(module: EExTestWeb.PageView, function: :__resource__, arity: 0) label(10) move(source: :page, destination: x(0)) return() label(11) line(1) func_info(module: EExTestWeb.PageView, function: :__templates__, arity: 0) label(12) move(source: {"lib/eex_test_web/templates/page", "*", ["index.html"]}, destination: x(0)) return() label(13) line(2) func_info(module: EExTestWeb.PageView, function: :render, arity: 1) label(14) move(source: %{}, destination: x(1)) call_only(&EExTestWeb.PageView.render/2) label(15) line(2) func_info(module: EExTestWeb.PageView, function: :render, arity: 2) label(16) is_atom(fail_label: 17, argument: x(0)) move(source: %{}, destination: x(2)) line(2) call_ext_only(&Phoenix.View.render/3) label(17) is_binary(fail_label: 18, argument: x(0)) jump(label: 19) label(18) allocate(words_of_stack: 0, live_x_register_count: 1) line(2) call_ext(&Kernel.inspect/1) line(2) gc_bif1(fail_label: 0, live_x_register_count: 1, import: &:erlang.byte_size/1, argument: x(0), destination: x(1)) bs_add(fail_label: 0, source1: x(1), source2: 47, unit: 1, destination: x(1)) bs_init2(fail_label: 0, size: x(1), words_of_stack: 0, live_x_register_count: 1, flags: 0, destination: x(1)) bs_put_string('render/2 expects template to be a string, got: ') bs_put_binary(fail_label: 0, size: :all, unit: 8, flags: 0, source: x(0)) move(source: x(1), destination: x(0)) line(2) call_ext(&ArgumentError.exception/1) line(2) call_ext(&:erlang.error/1) label(19) is_map(fail_label: 20, argument: x(1)) jump(label: 21) label(20) allocate(words_of_stack: 1, live_x_register_count: 2) move(source: x(0), destination: y(0)) move(source: x(1), destination: x(0)) line(2) call_ext(&Map.new/1) move(source: x(0), destination: x(1)) move(source: y(0), destination: x(0)) call_last(arity: 2, label: 16, deallocate_words_of_stack: 1) label(21) call_only(&EExTestWeb.PageView.render_template/2) label(22) line(1) func_info(module: EExTestWeb.PageView, function: :render_template, arity: 2) label(23) is_eq_exact(fail_label: 24, left: x(0), right: 6) move(source: x(1), destination: x(0)) call_only(&EExTestWeb.PageView."index.html"/1) label(24) is_map(fail_label: 27, argument: x(1)) get_map_elements(fail_label: 25, source: x(1), key_to_destination: list(:render_existing, x(2))) is_tagged_tuple(fail_label: 25, source: x(2), arity: 2, tag: EExTestWeb.PageView) get_tuple_element(source: x(2), element_number: 1, destination: x(4)) is_eq_exact(fail_label: 25, left: x(4), right: x(0)) move(source: nil, destination: x(0)) return() label(25) get_map_elements(fail_label: 26, source: x(1), key_to_destination: list(:template_not_found, x(2))) is_eq_exact(fail_label: 26, left: x(2), right: EExTestWeb.PageView) move(source: x(1), destination: x(2)) move(source: x(0), destination: x(1)) move(source: EExTestWeb.PageView, destination: x(0)) line(1) call_ext_only(&Phoenix.Template.raise_template_not_found/3) label(26) is_map(fail_label: 27, argument: x(1)) line(1) put_map_assoc(fail_label: 0, source: x(1), destination: x(1), live_x_register_count: 2, field_from_source: list(:template_not_found, EExTestWeb.PageView)) call_only(&EExTestWeb.PageView.template_not_found/2) label(27) test_heap(words_of_heap: 3, live_x_register_count: 2) put_tuple(size: 2, destination: x(0)) put(:badmap) put(x(1)) line(1) call_ext(&:erlang.error/1) label(28) line(2) func_info(module: EExTestWeb.PageView, function: :template_not_found, arity: 2) label(29) move(source: x(1), destination: x(2)) move(source: x(0), destination: x(1)) move(source: EExTestWeb.PageView, destination: x(0)) line(2) call_ext_only(&Phoenix.Template.raise_template_not_found/3) label(30) line(3) func_info(module: EExTestWeb.PageView, function: :"index.html", arity: 1) label(31) save_cp() move(source: "Welcome to %{name}!", destination: x(2)) move(source: "default", destination: x(1)) move(source: [{:name, "Phoenix"}], destination: x(3)) move(source: EExTestWeb.Gettext, destination: x(0)) line(4) call_ext(&Gettext.dgettext/4) is_tagged_tuple(fail_label: 32, source: x(0), arity: 2, tag: :safe) get_tuple_element(source: x(0), element_number: 1, destination: x(0)) jump(label: 34) label(32) is_binary(fail_label: 33, argument: x(0)) line(3) call_ext(&Plug.HTML.html_escape_to_iodata/1) jump(label: 34) label(33) line(4) call_ext(&Phoenix.HTML.Safe.to_iodata/1) label(34) test_heap(words_of_heap: 7, live_x_register_count: 1) put_list(head: x(0), tail: ["\n

A productive web framework that
does not compromise speed and maintainability.

\n
\n\n
\n
\n

Resources

\n \n
\n\n
\n

Help

\n \n
\n
\n"], destination: x(1)) put_list(head: "
\n

", tail: x(1), destination: x(1)) put_tuple(size: 2, destination: x(0)) put(:safe) put(x(1)) restore_cp() return() label(35) line(0) func_info(module: EExTestWeb.PageView, function: :module_info, arity: 0) label(36) move(source: EExTestWeb.PageView, destination: x(0)) line(0) call_ext_only(&:erlang.get_module_info/1) label(37) line(0) func_info(module: EExTestWeb.PageView, function: :module_info, arity: 1) label(38) move(source: x(0), destination: x(1)) move(source: EExTestWeb.PageView, destination: x(0)) line(0) call_ext_only(&:erlang.get_module_info/2) ```

Line chunk

Screen Shot 2019-07-22 at 7 48 27 AM

Screen Shot 2019-07-22 at 7 48 37 AM

KronicDeth commented 5 years ago

It's not just the debug-info (i.e. Dbgi chunk). The stacktrace needs to also reference the .eex files. Unless you're going to have the stacktrace support read the debug-info.

hansihe commented 5 years ago

I am not really using BEAM terminology here, by debug info I mean everything used at runtime used to refer back to originating source code.

This issue is really more about internal implementation than that stuff though.

bitwalker commented 5 years ago

So for stepping through code, we'll want to reference the actual source spans when generating debug info (i.e. DWARF). We always want to ignore -file there.

However, for compilation and runtime stacktraces/metadata, we'll want to use whatever is provided by the -file directive, since as pointed out, this is used intentionally to provide source errors in a way more useful to users of some module/header. In Erlang this is primarily used in -include'd files so that errors point to the included header, not the source file doing the including. In Elixir, this is used in situations like the one @KronicDeth shared, i.e. with macros, to make sure that location information points users to their own module rather than whatever module the macro came from.

As for how to store these diagnostics, I agree that we need a way to store both, but don't have a specific implementation in mind. Ideally we could access both from a single span, but that is probably overly expensive in the general case, since only a subset of code will have overridden source positions.

bitwalker commented 5 years ago

That said, might make sense to store both initially, and worry about optimizing it later.