fix(module_compilation): add dep to original source file #12774
Conversation
anmonteiro
force-pushed
the
anmonteiro/source-preview-ppxed
branch
from
8114ac4 to
ba8200c
Compare
|
|
||
| let source_without_pp ~ml_kind t = | ||
| let source = | ||
| match (ml_kind : Ml_kind.t) with |
There was a problem hiding this comment.
I think this might need an adjustment for sources without mli's. That is, when ml_kind = Intf but there's not t.source.files.intf we should fall back to the .impl.
Would be great to have a test for this as well if you have the time.
There was a problem hiding this comment.
Thanks, will have a look.
There was a problem hiding this comment.
hey @rgrinberg I've expanded the test case to account for .mli files as well, but I think I'm confused about your suggestion here: when's ever the case that we want to find source files when compiling interfaces?
There was a problem hiding this comment.
I've merged this to unblock #12688 but open to fixing once I have a better understanding.
There was a problem hiding this comment.
I think you have a point. When there's no explicit mli, and there's an error message to display, it will always be in the .ml anyway.
|
@anmonteiro Can you say a few words what this change is needed? |
|
Here is the reproduction case: #12770 |
Thanks! |
|
@nojb any other questions or concerns about this change or its implementation that I should consider? |
src/dune_rules/module_compilation.ml
Outdated
| ; A "-c" | ||
| ; Command.Ml_kind.flag ml_kind | ||
| ; Dep src | ||
| ; Hidden_deps (Dep.Set.of_files (Option.to_list original)) |
There was a problem hiding this comment.
Would be nice to leave a comment why we need the source file here. It is in fact only needed for sandboxed builds as far as I can tell.
Nothing from my side, thanks! |
Signed-off-by: Antonio Nuno Monteiro <[email protected]>
Signed-off-by: Antonio Nuno Monteiro <[email protected]>
anmonteiro
force-pushed
the
anmonteiro/source-preview-ppxed
branch
from
ba8200c to
4ea04ec
Compare
4 participants
No description provided.