Awesome job on the PR! On Fri, May 15, 2020 at 23:09 Dallin Osmun <dos...@gmail.com> wrote:
> Hi José > > Thank you so much for taking the time to respond to this thread. It has > been incredibly helpful and is very much appreciated! > > I think I may have found an issue with how line numbers are determined for > exceptions raised in macros. I submitted a PR here: > https://github.com/elixir-lang/elixir/pull/10040 > > > On Friday, May 15, 2020 at 2:29:48 PM UTC-6, José Valim wrote: >> >> Given Elixir compile time is really executing code, then I think it would >> be a nice feature to attempt to extract it from exceptions, although I >> understand it can be brittle. >> >> If the error is really a compile-time reason (invalid syntax, invalid >> call, etc), then Elixir does show a proper error with file:line. >> >> On Fri, May 15, 2020 at 9:30 PM cward <cade...@gmail.com> wrote: >> >>> It seems unreasonable for the editor to infer the line number from the >>> `message` key. Am I wrong in thinking that? It seems that `position` is >>> getting lost at some point in translation. Why would the `message` string >>> contain the full stacktrace with line number, but `position` is `nil`? >>> >>> After reading this thread, I now realize that this is actually somewhat >>> common. I don't have a use case off the top of my head, but it's often that >>> the `position` key is lost when an error comes from a macro. >>> >>> In the case of elixir_ls (vscode language server), the position is >>> expected, and the fallback is to highlight the whole file: >>> https://github.com/elixir-lsp/elixir-ls/blob/master/apps/language_server/lib/language_server/build.ex#L63 >>> >>> On Friday, May 15, 2020 at 12:30:52 PM UTC-6, José Valim wrote: >>>> >>>> You can use reraise/3. It is a bit misnamed, but it is correct. But >>>> this behaviour in particular sounds like a bug in the editor itself, given >>>> it does have a line in the stacktrace specific to that file. >>>> >>>> On Fri, May 15, 2020 at 8:04 PM Dallin Osmun <dos...@gmail.com> wrote: >>>> >>>>> I must be missing something. I am already using Macro.Env.stacktrace >>>>> when emitting my warnings and that's working great. The problem happens >>>>> when calling raise. Is there a way to pass a new stacktrace into raise? >>>>> The >>>>> docs for Kernel.raise say it accepts either a message or an exception and >>>>> attributes. Kernel.reraise takes in a stacktrace but when I use that I >>>>> only >>>>> see changes in the compiler diagnostic's message; not in it's file nor >>>>> position. >>>>> >>>>> >>>>> On Friday, May 15, 2020 at 11:45:10 AM UTC-6, José Valim wrote: >>>>>> >>>>>> 1. Yes, the granularity you get is per macro. >>>>>> >>>>>> 2. When emitting the warning, do "Macro.Env.stacktrace(__CALLER__)" >>>>>> to get the actual caller. You may also need to look at the AST to get the >>>>>> proper line. Note you would need to do the same if we were to introduce >>>>>> some sort of IO.error. >>>>>> >>>>>> On Fri, May 15, 2020 at 6:22 PM Dallin Osmun <dos...@gmail.com> >>>>>> wrote: >>>>>> >>>>>>> Hi José, >>>>>>> >>>>>>> Thanks for the helpful response! >>>>>>> >>>>>>> What you are saying makes sense. I must admit I don't have a very >>>>>>> advanced knowledge of metaprogramming in elixir so I'll have to >>>>>>> learn/practice a lot more before I can come up with an informed >>>>>>> response. >>>>>>> >>>>>>> I tried out your suggestion and wanted to report back on how it >>>>>>> went. Using IO.warn to report problems worked well. However, two issues >>>>>>> arose when I added this line to the end of the macro: >>>>>>> >>>>>>> raise "There are errors in your query" >>>>>>> >>>>>>> >>>>>>> Here is the code in question: >>>>>>> >>>>>>> >>>>>>> defmodule Queries do >>>>>>> use MyGoblet >>>>>>> >>>>>>> query "First" do >>>>>>> error1 >>>>>>> error2 >>>>>>> end >>>>>>> >>>>>>> query "Second" do >>>>>>> error3 >>>>>>> error4 >>>>>>> end >>>>>>> end >>>>>>> >>>>>>> >>>>>>> 1. The first call to the macro ran, reported warnings, and then >>>>>>> raised, halting compilation. error3 and error4 were then never reported. >>>>>>> This can be resolved by calling raise in an @after_compile hook but I >>>>>>> fear >>>>>>> doing so may suffer from the same issues you described above. Unless you >>>>>>> have any other suggestions I think that this issue, while not ideal, is >>>>>>> still acceptable. >>>>>>> >>>>>>> 2. Calling raise inside the macro caused the entire file in my >>>>>>> editor to turn red which made it next-to-impossible to see the original >>>>>>> warnings (see the attached screenshot). This was the diagnostic that was >>>>>>> reported from elixir: >>>>>>> >>>>>>> >>>>>>> %Mix.Task.Compiler.Diagnostic{ >>>>>>> compiler_name: "Elixir", >>>>>>> details: nil, >>>>>>> file: "/Users/dallin/code/goblet/lib/testing.ex", >>>>>>> message: "** (RuntimeError) There are errors in your query\n >>>>>>> (goblet 0.1.0) lib/goblet.ex:74: Goblet.build/6\n expanding macro: >>>>>>> MyGoblet.query/2\n lib/testing.ex:8: Queries (module)\n", >>>>>>> position: nil, >>>>>>> severity: :error >>>>>>> } >>>>>>> >>>>>>> >>>>>>> Even though the stack trace has a line number in it, that position >>>>>>> is missing at the root of the diagnostic. This is what ultimately causes >>>>>>> the issue. Would you like me to file this as a bug in the elixir issue >>>>>>> tracker? Or should I instead work with the authors of the editor plugin >>>>>>> to >>>>>>> handle position-less Diagnostics differently? >>>>>>> >>>>>>> [image: screenshot.png] >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Wednesday, May 13, 2020 at 12:20:29 PM UTC-6, José Valim wrote: >>>>>>>> >>>>>>>> Hi Dallin, >>>>>>>> >>>>>>>> Thank you for the detailed proposal. >>>>>>>> >>>>>>>> It is important to remember that in Elixir compilation and >>>>>>>> execution steps are not necessarily distinct. For example, someone >>>>>>>> could >>>>>>>> have this project: >>>>>>>> >>>>>>>> # lib/a.ex >>>>>>>> defmodule A do >>>>>>>> use UsesYourMacro >>>>>>>> query do >>>>>>>> ... >>>>>>>> end >>>>>>>> end >>>>>>>> >>>>>>>> # lib/b.ex >>>>>>>> defmodule B do >>>>>>>> A.query >>>>>>>> end >>>>>>>> >>>>>>>> In other words, B can already try to execute the code from A before >>>>>>>> compilation finishes. Therefore, "error" can be misleading because >>>>>>>> continuing compilation with an error can lead to cascading failures, >>>>>>>> and >>>>>>>> causing the user to chase a bug that's not the original one. >>>>>>>> >>>>>>>> At best, you would need to consider mixing "IO.error" and "raise". >>>>>>>> My suggestion in your case is to emit warnings for all errors in a >>>>>>>> particular query definition, allowing the user to get multiple >>>>>>>> reports, and >>>>>>>> then raise at the end of that query definition if any warning was >>>>>>>> emitted. >>>>>>>> >>>>>>>> This doesn't provide warnings across all files but it can be a good >>>>>>>> starting point, especially because we indeed cannot continue beyond >>>>>>>> that >>>>>>>> file for the reasons I mentioned above. >>>>>>>> >>>>>>>> >>>>>>> -- >>>>>>> You received this message because you are subscribed to the Google >>>>>>> Groups "elixir-lang-core" group. >>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>> send an email to elixir-l...@googlegroups.com. >>>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/d/msgid/elixir-lang-core/ad4e81d8-6495-4511-ad7b-f6064dd1a95c%40googlegroups.com >>>>>>> <https://groups.google.com/d/msgid/elixir-lang-core/ad4e81d8-6495-4511-ad7b-f6064dd1a95c%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>> . >>>>>>> >>>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "elixir-lang-core" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to elixir-l...@googlegroups.com. >>>>> To view this discussion on the web visit >>>>> https://groups.google.com/d/msgid/elixir-lang-core/704b0050-1e98-4322-b63f-34ebdc36e324%40googlegroups.com >>>>> <https://groups.google.com/d/msgid/elixir-lang-core/704b0050-1e98-4322-b63f-34ebdc36e324%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>> . >>>> >>>> >>>>> -- >>> You received this message because you are subscribed to the Google >>> Groups "elixir-lang-core" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to elixir-l...@googlegroups.com. >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/elixir-lang-core/d92d2a86-2b27-406f-9d70-a96600fc51ed%40googlegroups.com >>> <https://groups.google.com/d/msgid/elixir-lang-core/d92d2a86-2b27-406f-9d70-a96600fc51ed%40googlegroups.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- > You received this message because you are subscribed to the Google Groups > "elixir-lang-core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to elixir-lang-core+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/elixir-lang-core/92e66d66-8ab6-44e1-a69c-33a82b88d791%40googlegroups.com > <https://groups.google.com/d/msgid/elixir-lang-core/92e66d66-8ab6-44e1-a69c-33a82b88d791%40googlegroups.com?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "elixir-lang-core" group. To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4K440wNvAJkorZN5CmifvBaVNaLk6bJQMVt3WiU3tndLQ%40mail.gmail.com.
