[m-rev.] for review: ho_inst_info clarification
jfischer at opturion.com
Thu Nov 12 09:53:26 AEDT 2015
On Sun, 8 Nov 2015, Mark Brown wrote:
> I've gone through all of the uses of ho_inst_info 'none' in the
> compiler and identified places I suspect may be a problem, and marked
> them with an XXX comment. I haven't tried to fix anything in this
> change; I'll look at fixing inst_match.m, inst_util.m and
> modecheck_unify.m in separate changes. The problems in inst_match.m
> look to be responsible for mantis bug 264.
> The XXX comments in compiler/float_regs.m are for review by Peter.
> Suggestions welcome for an alternative to the new name none_or_standard_func.
> Change 'none' to 'none_or_standard_func'
I suggest 'none_or_default_func' since the term "default function mode"
is used pretty much throughout the system and introducing a second term
to describe the same concept is potentially confusing.
> in ho_inst_info. This value
> is used for first-order values as well as for functions that have the
> standard inst, and the new name better reflects that.
> Places using this value have been checked for correctness. Issues that
> need to be looked at have been marked with XXX, although have not been
> addressed in this change.
> Update the type.
> Add XXX comments. This module needs to check for non-standard
> function insts in a bunch of places.
> Add XXX comments. We lose information about non-standard function
> insts when merging bound and any. The information needs to be
> either preserved or disallowed entirely.
> Add XXX comments. This module may miss cases involving standard
> function insts.
> Add XXX comment. We should exclude pred and non-standard func
> lambda non-locals from becoming locked, an addition to standard
> No special handling is required for other modules.
The diff looks fine. Others may have comments regarding the individual XXXs,
but that shouldn't prevent you committing.
More information about the reviews