[erlang-patches] Print column numbers when compiling

Anthony Ramine <>
Sun Sep 2 20:26:24 CEST 2012


Hi,

Again, see responses inlined; please refetch.

Regards.

--
Anthony Ramine

Le 28 août 2012 à 15:28, Lukas Larsson a écrit :

> Hello,
> 
> See responses inlined.
> 
> Lukas
> 
> On 24/08/12 12:22, Anthony Ramine wrote:
>> Most OTP (at least erl_lint, erl_parse and syntax_tools) already knows that locations can be a line or a line and a column; furthermore it is said in erl_parse itself that one should always use get_line/1 and set_line/3 instead of depending on having an integer there.
> It does indeed say so inside the code of erl_parse. But all of the visible documentation (here[1] and here[2]) say that it is an integer. So if you have only read the documentation (which I assume most people only have) you would assume that it is an integer. That documentation has to be updated so that LINE can be either an integer() or a tuple(integer(),integer()), and a reference to use erl_parse:set/get_attribute to manipulate it safely.
> 
> The compiler should also be updated to use these API's, this includes the convention that a line number of -1 is used to say that something is compiler generated. To make sure that all uses of the non-api is eliminated, it might be a good idea to declare erl_scan:line as -opaque and let dialyzer search through the otp code for errors.
> 
> [1]: http://www.erlang.org/doc/apps/erts/absform.html
> [2]: http://www.erlang.org/doc/man/erl_scan.html#type-line

I replaced LINE by LOC in the first documentation you point to and I've patched various type specs in erl_scan and erl_parse so they both use location().

With regard to negative line numbers, it should be noted that user-supplied -file attributes have their line numbers negated whereas as you said in other modules like erl_lint and v3_core, compile-generated nodes are the ones which lines are negated. Shouldn't the opposite be done in epp's -file attributes handling?

>> If it is really that heavy of a change, why do we have erl_scan:set_attribute/3 and erl_scan:attributes_info/{1,2}? They obviously were coded to mitigate these issues.
> We have indeed been working on getting this to work before, but the effort was left as you see it now because there were a number of issues which needed solving and more important things got in the way. The reason why I say it is a heavy change is that we want to feel that a good effort has been made to make sure that all error and warning column numbers point to the correct position.

I've already added a 'file' item to these functions to improve the code populating every node with the file from the -file attributes of the module declaration in erl_lint; maybe another 'generated' item should be added to replace the multiple neg_line and abs_line functions there are in some compile-related modules?

>> Any parse transform that break because of this change will break because it does AST manipulation wrong and their users will just have to not use the "column" option for it to continue to work.
> Strictly speaking parse transform is also an experimental/undocumented feature of Erlang/OTP. But unfortunately a lot of people seem to use them, so we have to take some backwards compatibility into consideration. The closest thing to a documentation is the example found here lib/stdlib/example/erl_id_trans.erl, and that does not deal with line numbers at all.
> 
> We really really have to be sure that the fun2ms and qlc parse transforms work.

I've read through the code of various parse transforms this week and they don't seem to make anything related to the transformed nodes' locations. Most OTP-provided parse transforms are covered in the test suites I patched.



More information about the erlang-patches mailing list