Re: [elixir-core:9482] Re: Proposal: IO.error/2

2020-05-15 Thread José Valim
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  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\nexpanding macro:
> MyGoblet.query/2\nlib/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-lang-core+unsubscr...@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
> 
> .
>

-- 
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/CAGnRm4%2BV6DppveoWxwdU1fOFuvOaTFSfptE5mtABrC562naqyg%40mail.gmail.com.


Re: [elixir-core:9482] Re: Proposal: IO.error/2

2020-05-15 Thread Dallin Osmun
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\nexpanding macro: 
MyGoblet.query/2\nlib/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-lang-core+unsubscr...@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.