Bugfix var local name

[yannilefki]

Fixed the span bug with Variable.local_name.
For that I :

• Created a new file "seq_helper.ml" to solve module dependencies
• Added a handling of the Dir_span case in two functions of "path.ml"

p.s. : the modification of ShowAt is to solve another documented dependency issue, which has to link to this branch.

[Bastacyclop]

it's good practice to extend the local name unit test with a few cases that test the span feature.

[Bastacyclop]

This is an interesting solution, so basically if you are targeting a span, you create a sub-sequence on-the-fly so that transformations may treat is as a regular sequence node without explicit treatment. Note that this is not how we programmed similar transformations, see assert_instr_effects_shadowed Accesses_basic.transform_on, Function_core.uninline_on`, ... (grep 'update_span_helper'). The question would be : what are the pros and cons to having transformations explicitly manipulate spans vs the target system manipulating spans transparently.

[yannilefki]

In the end I did not modify to trm_seq_nobrace.
The No_brace modules asks for any nobrace sequence to be wrapped with a Nobrace_transfo.remove_after, which would be difficult in this situation, since this function is called for its output.
To be more precise, taking the unittest I wrote as an example, I would get an execution failure from Nobrace.current, because there is no live 'id' to use as a Nobrace annotation.
For me to be able to wrap with Nobrace_transfo.remove_after, I would need a 'unit -> unit' function, this means that I would need to encapsulate many function calls inside, which I believe is not a good idea.

[Bastacyclop]

then you should try a unit test where you call the transformations on a subsequence containing an allocation, which probably won't work:

/* @target-begin */
int a = 0;
b++;
/* @target-end */
a++;

in this case it seems like creating sub-sequences on-the-fly during target resolution, although tempting because unifying APIs, might not be the way to go.

[Bastacyclop]

One could also consider keeping it qualified but short as in Seq_helper.update_span. In general, I think our current codebase is doing too much module opening and namespace flattening.

[yannilefki]

I don't think I understand your point? Do you mean by that that function names are too long? You are right that we could for example change from Seq_helper.update_span_helper to Seq_helper.update_span, as it would not change the meaning of the function's name.

[Bastacyclop]

I mean that we use ad-hoc prefixes like trm_add instead of nice modules like Trm.add that act as prefixes just as well.

[Bastacyclop]

I think part of the point for this test was to speed-up the span.start = span.stop no-op case.

[Bastacyclop]

try doing more complex spans that don't start/end exactly on a sequence, and that contain allocations whose scope matters.