-
Notifications
You must be signed in to change notification settings - Fork 412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generate .cmt files in native compilation #3758
Conversation
Signed-off-by: Cameron Wong <cwong@janestreet.com>
Signed-off-by: Cameron Wong <cwong@janestreet.com>
Signed-off-by: Cameron Wong <cwong@janestreet.com>
Gah, just kidding, |
Looks like this feature is going to take a bit longer than I expected -- right now, The three paths forward that I see are as follows:
|
I'd like to revisit the discussion of this issue before more effort is spent on this. The author never clarified why he disabled bytecode builds in the first place. If it was for a reason like "optimizing" the build, then I don't think this issue is worth addressing. |
Understood; I'll shelve this for the time being. Perhaps @jeremiedimino will have thoughts once he returns. |
Sorry about not warning you earlier. I had this issue assigned earlier and arrived at similar hacks as in this PR. In the end, I struggled to justify adding so much crud to address address a non-issue for the vast majority of users. Not to mention that adding |
oof, misclicked the review request button, sorry! |
Note that for the related issue #3467, the root motivation is not about the bytecode compiler not being available. There is a goal to make every warning/error from the compiler appear exactly once. Enabling both byte and native modes leads to duplicated warnings and errors. Turning off warnings in ocamlc_flags doesn't work since .mli files are only compiled with ocamlc. So for that particular case, a way to pass different flags to ocamlc when compiling .ml and .mli would be a workaround. (Also for the record, without thorough benchmarks I don't think that just dismissing the build time of bytecode as irrelevant is necessarily justified. Compiling to bytecode still has to perform type inference, which for some code is not trivial.) |
Thanks for helping me understand the issue @jberdine.
I think that disabling bytecode builds to solve this problem is a bad workaround. Byte code builds are incredibly useful for many reasons (debug, toplevel, jsoo) and it's not practical to disable them for most of us.
I'm dismissing this workaround rather than the issue. If type checking is so expensive, you don't have to use By the way, both of these problems can be solved by improving the compiler to support a separate type checking phase that produces cmi, cmt, etc. This solution would solve the double warnings and improve build times. Would be great to explore this solution rather than pile on band aids in dune. |
I agree that disabling bytecode compilation is a bit of a crude club-style workaround, and it may not be worth lots of hacks in dune, but in that case I think that the I had the understanding from long ago, probably now-stale information, that at least part of the problem is that there are multiple ways to generate e.g. .cmi from .mli, either using ocamlc or ocamlopt. My understanding is that because of reasons (to do with build systems liking obviously commuting graphs or something), dune chooses to only use one of those possible paths from .mli to .cmi, leading to e.g. invocation of ocamlc even when producing only native code. Finding a way around that sort of situation seems like it might be solvable in the land of the build system without resorting to hacky workarounds. Maybe. I don't know if the situation with .cmt files is analogous or not. But I agree that being able to stop/continue after typing would be best. Implementing that feels like it could share a lot with the |
After some further discussion, I think we've decided that, while everyone's concerns are valid, this particular approach is probably not the correct one for dune. Moving forward, we are planning to submit a compiler patch to be able to resume compilation from Pending performance testing, this should resolve many issues much more nicely than this approach. |
As per discussion in #3182.
It's a little sloppy, but I think it works.