Give compiler hints when handling run-time deps in parse_transforms #2606
Conversation
Assume that the original app order respects the dependency order as supplied by the rebar3 dependency order. Then what we do is interleave the hard compile- time dependencies as found by analysis to maintain their ordering constraints into those given by the dependency order. This should lower the chance of hitting conflicts when runtime dependencies become compile-time dependencies via interactions in parse transform calls.
As described in erlang#2563, part of the previous approach's weakness is that we need to seed the compile order with the dependency order for some invisible compile-time dependencies (runtime ones turned to build-time ones via calls to parse-transforms). Unfortunately, topological sorting is insufficient due to how it flattens apps. Assume the DAG contains: a -> b c -> d then a valid topological sort would be `[a,c,b,d]`. However, if we introduce the hidden dep (and decide that `c` calls `b`in a parse_transform) that the DAG doesn't contain, then the sorting order is broken. However, if we initially carried the `[x,b,a,c,f,e,d]` dep list, we can use a run over it to build `[x,a,b,c,f,e,d]` final compile order.
|
I'm not quite sure how to set up a test for this, but may add a small unit one. |
|
For tests, @tothlac has an example project, right? Could we add that whole thing to rebar3_tests? |
|
It has some private content that required sharing it privately but I can see if I can extract a minimal case |
And fixed in erlang/rebar3#2606 Relies on having all relevant libs compiled in the same phase (i.e. all top-level apps or all deps), with two layers of transitive libraries depending on parse_trans, which is called at runtime during the compiler phase. If the compiler does not feed dep-level information and does no runtime analysis of compile time deps, either a_lib or z_lib will fail to build when b_lib calls parse_trans (depending on sorting order).
|
@tsloughter added in tsloughter/rebar3_tests#8 |
src/rebar_compiler_dag.erl
Outdated
| []; | ||
| interleave([App|Apps], DAG, Expanded) -> | ||
| case Expanded of | ||
| #{App := _} -> |
There was a problem hiding this comment.
Could you be using the guard is_map_key in interleave and dedupe instead?
There was a problem hiding this comment.
Actually, why is a map used, is Expanded not just acting as a set?
There was a problem hiding this comment.
it's acting as a set, but the map is generally faster than the set. I can swap the datastructure if you want though.
There was a problem hiding this comment.
Ah, ok, I'm fine either way. I did recall discussion about switching some of these data structures internally to use maps and that lead me to seeing sets has a "version 2", https://erlang.org/doc/man/sets.html#new-1
Since we aren't doing any set operations besides is_element I don't think it is too important to switch, but seems we can in the future and possibly not lose speed.
There was a problem hiding this comment.
Yeah I'm good going for the semantics and using the sets module then. Will update later today.
Assume that the original app order respects the dependency order as supplied by
the rebar3 dependency order. Then what we do is interleave the hard compile-
time dependencies as found by analysis to maintain their ordering constraints
into those given by the dependency order.
This should lower the chance of hitting conflicts when runtime dependencies
become compile-time dependencies via interactions in parse transform calls.
As described in #2563,
part of the previous approach's weakness is that we need to
seed the compile order with the dependency order for some
invisible compile-time dependencies (runtime ones turned
to build-time ones via calls to parse-transforms).
Unfortunately, topological sorting is insufficient due to how it
flattens apps. Assume the DAG contains:
then a valid topological sort would be
[a,c,b,d]. However, if weintroduce the hidden dep (and decide that
ccallsbin aparse_transform) that the DAG doesn't contain, then the sorting order
is broken.
However, if we initially carried the
[x,b,a,c,f,e,d]dep list,we can use a run over it to build
[x,a,b,c,f,e,d]final compileorder.
This is unlikely to show up for project apps, where no such information
exists, but a similar hint can be given through include files under
the developer's control.