WIP: Reorganizing recompilation and iface writing checking
(It is a "WIP issue" so I can confer with @cale remotely.)
!3204 (closed) uses more explicit pattern matching in GHC.Driver.Make.hscIncrementalFrontend
, but the algorithm is still quite weird, and there are silly things like me adding two new very similar data types. #18205 also points out there is some weird instances where we have an old iface, but don't return it so that GHC.Driver.Make
has no old fingerprint, when one would think we there should be no harm in having an old fingerprint whenever there is an old iface, as the driver can still replace the old iface.
Assuming #18205 issue is fixed, there is further changes I'd like to do building on the !3204 (closed). Per !3215 (comment 270508) we currently check for an an old iface once, with rather complicated and yet still unsatisfactory logic. The problem is we are trying to simultaneously ask both:
-
Whether we should compile
-
Should we write an iface if we did compile.
We perhaps shouldn't try to decide all of that at once. Instead, we can:
-
Decide just 1 in
GHC.Driver.Make.hscIncrementalFrontend
. We can callcheckVersions
directly, inliningcheckOldIface
(andcheck_old_iface
), and not even bothering to look at the old iface if the source changed, which will simplify the inlined code. Also, I thinkm_tc_result
andsource_modified
must vary together, otherwise we'd be blatantly not type checking again if the source changed. If we combine those two arguments to reflect this, I think this code can be further simplified. -
Afterwards, pass along
data IfaceStatus = OldIfaceFingerprint Fingerprint | NoOldIface | IfaceNotYetChecked
instead of
Maybe Fingerprint
to the next thing called byGHC.Driver.Make.hscIncrementalFrontend
. -
After compilation, decide question 2 by changing:
when want_iface {- something about -fno-code and -fwrite-interface, etc. -} $ existing_logic_about_whether_to_write_iface ...
when want_iface $ do mb_old_fingerprint <- case ... of OldIfaceFingerprint old_fingerprint -> pure $ Just old_fingerprint NoOldIface -> pure Nothing IfaceNotYetChecked -> getFingerprint <$> checkForOldIface existing_logic_about_whether_to_write_iface mb_old_fingerprint
The
checkForOldIface
would again be made from inliningcheckOldIface
andcheck_old_iface
. Since we now don't need to worry about whether the source changed (if we got this far, it did) we can simplifycheckOldIface
in different ways than when we inlined it above.
There's a few other things that could be simplified after
There is also the question of the whether we need the outputting object files / --interactive
special case that led me to write (abanonded) !3215 (closed). This might also dealing with that in a more satisfactory manner too. This was already removed from the the first step because by never reading the iface if the source changed, we skip that special case entirely (essentially doing the fast --interactive
way all the time). It still would technically remain here, since as @osa1 points out in !3215 (comment 270433), that special case also has the affect of making us always replace the iface. Thankfully the bulk of step 3 is skipped we if we know we aren't writing interface files, so with that and the recompilation now out of the way, the decision now boils down to whether we should spend extra effort on iface deserialization to avoid the cost of iface serialization (when an iface already exists), which is a lot simpler than the original.
Also, if can move the EPS
modification somewhere else, we'll have enough uniformity in checkVersion
that we can dispense with the ExceptT
introduced in !3204 (closed). There is already a comment about this being redundant in --make
mode, so perhaps this code is already due to be relocated.
Finally, in !3215 (comment 270513) @osa1 mention that with #16885 (closed) we should be needing to examine iface files if the source didn't change less than we do, which I imagine will further simplify this code.