[m-rev.] Solver support for abstract equivalence solver types

Ralph Becket rafe at cs.mu.OZ.AU
Mon Dec 13 11:16:49 AEDT 2004


Julien Fischer, Monday, 13 December 2004:
> 
> On Wed, 1 Dec 2004, Ralph Becket wrote:
> 
> > Index: compiler/goal_util.m
> > ===================================================================
> >
> > +:- pred goal_util__generate_unsafe_casa(prog_var::in, prog_var::in,
> > +	(inst)::in, (inst)::in, prog_context::in, hlds_goal::out) is det.
> > +
> I think you should add a comment to this predicate about why the version
> with the two extra inst arguments is necessary.

I've added the following comments:

        % Generate an unsafe cast goal.  The input and output insts
        % are just ground.
        %
:- pred goal_util__generate_unsafe_cast(prog_var::in, prog_var::in,
        prog_context::in, hlds_goal::out) is det.

        % This version takes input and output inst arguments, which may
        % be necessary when casting, say, solver type values with inst
        % any, or casting between enumeration types and ints.
        % 
:- pred goal_util__generate_unsafe_cast(prog_var::in, prog_var::in,
        (inst)::in, (inst)::in, prog_context::in, hlds_goal::out) is det.

> > Index: compiler/polymorphism.m
> > ===================================================================
> >
> > +	% Extract some fields from a pred_info and proc_info and use them to
> > +	% create a poly_info, for use by the polymorphism transformation for
> > +	% transforming a new call goal.
> > +:- pred create_poly_info_for_call(module_info::in, pred_info::in,
> > +	proc_info::in, prog_varset::in, vartypes::in, poly_info::out) is det.
> > +
> Wouldn't create_poly_info_for_new_call be a more appropriate name for
> this?

Done.

> I'm a bit dubious about how well these changes to the polymorphism
> module will work when there are calls to predicates whose arguments
> have types that have existentially quantified data constructors that
> have typeclass constraints attached to them.
> 
> As far as I'm aware the whole matter of solver types and exisential
> types, typeclasses etc hasn't really been considered at the moment.
> You should put a big XXX comment over these predicates saying that
> they are only for use with the code that handles solver types.

I've added the following XXX to the log file and the comments for the
declaration and implementation:

% XXX This predicate does not yet handle calls whose arguments include 
% existentially quantified types or type class constraints. 

> > +
> > +		% Work out how many type_info args we need to prepend.
> > +		%
> > +	NCallArgs0 = length(CallArgTypes0),
> > +	NPredArgs  = length(PredArgTypes),
> I'd module-qualify those calls to length. It shouldn't matter but
> intermodule optimization sometimes complains about type ambiguities if
> you don't.

Done.  Although that is surely a bug in IMO.

> > +		TypeBody = eqv_type(EqvType)
> > +	->
> > +		goal_info_init(Context, GoalInfo),
> > +		unify_proc__make_fresh_named_var_from_type(EqvType,
> > +			"PreCast_HeadVar", 1, X0, !Info),
> > +		(
> > +			type_to_ctor_and_args(EqvType, TypeCtor0, _TypeArgs)
> > +		->
> > +			TypeCtor = TypeCtor0
> > +		;
> > +			error("unify_proc__generate_initialise_clauses: " ++
> > +				"type_to_ctor_and_args failed")
> > +		),
> > +		PredName = special_pred__special_pred_name(initialise,
> > +				TypeCtor),
> Fix the indentation here.
> 
> > +		hlds_module__module_info_name(ModuleInfo, ModuleName),
> > +		TypeCtor = TypeSymName - _TypeArity,
> > +		sym_name_get_module_name(TypeSymName, ModuleName,
> > +			TypeModuleName),
> > +		InitPred = qualified(TypeModuleName, PredName),
> > +		PredId   = invalid_pred_id,
> > +		ModeId   = invalid_proc_id,
> > +		InitCall = call(PredId, ModeId, [X0], not_builtin, no,
> > +				InitPred),
> > +		InitGoal = InitCall - GoalInfo,
> and here.

Sorry, what's the problem with the indentation here?

> That (along with the other bugfix) looks alright otherwise.  Can you
> post the test cases for these changes before you commit.

Coming up...  Thanks for reviewing this.

-- Ralph
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------



More information about the reviews mailing list