[m-rev.] For review: new constraint based mode analysis

[Richard James FOTHERGILL]

On Tue, 8 Feb 2005, David Overton wrote:

>>
>> compiler/new_mode_constraints.m
>
> This is not a good name for the module, since it presumably won't be new
> for long.
>

Good point. I'll think on it.

>> 	This module serves as an interface between the existing
>> 	mode_constraints module which asks for constraints for an SCC, and
>> 	build_mode_constraints, which only handles goals.  Simply put, it
>> 	takes
>> 	an SCC and hands it to build_mode_constraints one predicate at a
>> 	time.
>> 	It produces the mode constraints for that SCC, and the constraint
>> 	variables necessary to do so.
>
> There's something wrong with your line wrapping here.
>

So it would seem. My compose window wraps at 80 characters. Somewhere past
sending it it's been mangled. I'll make sure 80's ok for the log, or else
change it.


>> +% This module contains data structures designed for use with constraints
>> +% based mode analysis. They represent constraints between constraint
>> variables,
>> +% such as those one might use to describe where a program variable may be
>> +% produced.
>> +%
>
> Please fix up the line wrapping here and in other comments where
> necessary.
>

Keeping lines to 79 characters was supposed to avoid this happening. I'll
see what needs doing.


>> +% XXX change to mc_var throughout
>
> Why is this comment here?
>

Reminder note. Thanks for pointing it out.

>> +	% Represents conjunctions and disjunctions between atomic constraints
>> +	% on constraint variables.  The advantage of the constraints for this
>> +	% implementation of mode checking is that they can be expressed
>> almost
>> +	% entirely as variable to variable constraints, with little use for
>> the
>> +	% disj and conj functors of this structure.  It should be noted that
>> +	% the intention is to only use the conj constructor as a child of
>> disj
>
> Please clarify what you mean here: is the user of this module not
> permitted to use a conj constructor that is not a child of a disj?

I'll see what I can do with the comments. Use of 'conj' not as a child
of disj is permitted - even reccomended in the general sense. I was
refering to the fact that, when it comes to mode constraints, I've
omitted the conj constructor at the top level, and just used the
constraint formulae type. Probably I should just remove that part of
this comment - while 'conj' was only included because it was needed
for use as a child of 'disj', I suppose there's no reason to say so.



>> +	(	proc_info_maybe_declared_argmodes(ProcInfo, yes(Decl))
>> +	->	add_sufficient_in_modes_for_type_info_args(
>> +			CallArgs,
>> +			Decl,
>> +			FullDecl
>> +		),	% XXX type_info args should be at the start and
>> +			% should be 'in' so that is what this predicate adds
>> +			% however, I am not happy with it.
>
> Why?
>

I'm not happy with it because it relies on type info args being added in
the same manner indefinitely, and on no other arguments being added, or
else whoever makes changes magically finding this predicate to change
it accordingly. Also, someone made a passing comment to me that the mode
of type info args might not always be strictly 'in' (although presumably
it would always be a close variant of it).
Basically, I don't think I should be adding the arg mode info for the
type info arguments myself - I feel like I should be using information
that is added wherever the extra arguments are added. I used the maybe
declared argmodes field to make sure that the predicate actually
had modes declared. Its possible that the actual headmodes field has
the information I need. I didn't use it originally because I thought
it must be a field that gets filled in by mode analysis.


>> +	;	Constraints = []
>> +			% XXX Should this be an error?
>
> Yes, a foreign_proc without a mode declaration is an error.
>

Thanks.



Thanks for all that,
Richard
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------