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

Handle file directive correctly #18

Open
hansihe opened this issue Jul 22, 2019 · 8 comments
Open

Handle file directive correctly #18

hansihe opened this issue Jul 22, 2019 · 8 comments

Comments

@hansihe
Copy link
Member

hansihe commented Jul 22, 2019

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
Copy link
Member Author

hansihe commented Jul 22, 2019

Code isn't pushed just yet

@KronicDeth
Copy link
Contributor

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
Copy link
Member Author

hansihe commented Jul 22, 2019

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
Copy link
Contributor

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

<div class="jumbotron">
  <h2><%= gettext "Welcome to %{name}!", name: "Phoenix" %></h2>
  <p class="lead">A productive web framework that<br />does not compromise speed and maintainability.</p>
</div>

<div class="row marketing">
  <div class="col-lg-6">
    <h4>Resources</h4>
    <ul>
      <li>
        <a href="http://phoenixframework.org/docs/overview">Guides</a>
      </li>
      <li>
        <a href="https://hexdocs.pm/phoenix">Docs</a>
      </li>
      <li>
        <a href="https://github.com/phoenixframework/phoenix">Source</a>
      </li>
    </ul>
  </div>

  <div class="col-lg-6">
    <h4>Help</h4>
    <ul>
      <li>
        <a href="http://groups.google.com/group/phoenix-talk">Mailing list</a>
      </li>
      <li>
        <a href="http://webchat.freenode.net/?channels=elixir-lang">#elixir-lang on freenode IRC</a>
      </li>
      <li>
        <a href="https://twitter.com/elixirphoenix">@elixirphoenix</a>
      </li>
    </ul>
  </div>
</div>

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: ["</h2>\n  <p class=\"lead\">A productive web framework that<br />does not compromise speed and maintainability.</p>\n</div>\n\n<div class=\"row marketing\">\n  <div class=\"col-lg-6\">\n    <h4>Resources</h4>\n    <ul>\n      <li>\n        <a href=\"http://phoenixframework.org/docs/overview\">Guides</a>\n      </li>\n      <li>\n        <a href=\"https://hexdocs.pm/phoenix\">Docs</a>\n      </li>\n      <li>\n        <a href=\"https://github.com/phoenixframework/phoenix\">Source</a>\n      </li>\n    </ul>\n  </div>\n\n  <div class=\"col-lg-6\">\n    <h4>Help</h4>\n    <ul>\n      <li>\n        <a href=\"http://groups.google.com/group/phoenix-talk\">Mailing list</a>\n      </li>\n      <li>\n        <a href=\"http://webchat.freenode.net/?channels=elixir-lang\">#elixir-lang on freenode IRC</a>\n      </li>\n      <li>\n        <a href=\"https://twitter.com/elixirphoenix\">@elixirphoenix</a>\n      </li>\n    </ul>\n  </div>\n</div>\n"], destination: x(1))
      put_list(head: "<div class=\"jumbotron\">\n  <h2>", 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: ["</h2>\n  <p class=\"lead\">A productive web framework that<br />does not compromise speed and maintainability.</p>\n</div>\n\n<div class=\"row marketing\">\n  <div class=\"col-lg-6\">\n    <h4>Resources</h4>\n    <ul>\n      <li>\n        <a href=\"http://phoenixframework.org/docs/overview\">Guides</a>\n      </li>\n      <li>\n        <a href=\"https://hexdocs.pm/phoenix\">Docs</a>\n      </li>\n      <li>\n        <a href=\"https://github.com/phoenixframework/phoenix\">Source</a>\n      </li>\n    </ul>\n  </div>\n\n  <div class=\"col-lg-6\">\n    <h4>Help</h4>\n    <ul>\n      <li>\n        <a href=\"http://groups.google.com/group/phoenix-talk\">Mailing list</a>\n      </li>\n      <li>\n        <a href=\"http://webchat.freenode.net/?channels=elixir-lang\">#elixir-lang on freenode IRC</a>\n      </li>\n      <li>\n        <a href=\"https://twitter.com/elixirphoenix\">@elixirphoenix</a>\n      </li>\n    </ul>\n  </div>\n</div>\n"], destination: x(1))
      put_list(head: "<div class=\"jumbotron\">\n  <h2>", 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
Copy link
Contributor

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
Copy link
Member Author

hansihe commented Jul 22, 2019

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
Copy link
Member

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants