[m-rev.] for review: arg_vectors (part 1)

[Julien Fischer]

On Thu, 21 Dec 2006, Mark Brown wrote:

> On 18-Dec-2006, Julien Fischer <juliensf at csse.unimelb.edu.au> wrote:
>>
>> For review by Mark or Zoltan.
>>
>> Estimated hours taken: 14
>> Branches: main
>>
>> Add a new data structure, called an arg_vector, to the compiler that is
>> intended to encapsulate the argument passing conventions that we use.
>> (Primarily those described in the comments at the head of polymorphism.m).
>> The intention is to use this new data structure everywhere we currently use
>> lists to represent procedure and call site argument information.
>
> Could you list the other places that will eventually use this data structure?
> Is it only the proc_info, or are there others (e.g., the parse tree)?

No, in the HLDS the arg_types field in the pred_info is another obvious
candidate as are the argument lists in calls and foreign_calls in the
hlds_goal_expr type.

At the moment I was only intending to use this data structure in the HLDS.
Using it (or something similar) elsewhere is future work.

>>
>> This change adds the new data structure plus supporting utility predicates.
>> It also begins the task of moving the code in the compiler over to the new
>> data structure.  Specifically, this diff change the headvars field in the
>
> s/change/changes/

Fixed.

>> clauses_info structure into an arg_vector.
>>
>> The spots marked XXX ARGVEC will need to be further modified once the
>> proc_info structure has been modified to use arg_vectors.
>>
>> compiler/hlds_args.m:
>> 	New module.  This module defines the arg_vector structure that will
>> 	eventually be used to represent procedure and call site argument
>> 	information throughout the compiler.  It also defines some utility
>> 	procedures that operate on that data structure.
>>
>> compiler/hlds.m:
>> 	Include the new module in the HLDS package.
>>
>> compiler/hlds_clauses.m:
>> 	Modify the clauses_info structure to represent clause arguments as
>> 	arg_vectors rather than lists.
>>
>> 	Adapt the predicates that initialise the clauses_info structure so
>> 	that they correctly handle the arg_vector structure's representation
>> 	of function return values.
>>
>> compiler/polymorphism.m:
>> 	Use the arg_vector data structure when introducing type_infos and
>> 	typeclass_info arguments.
>
> Add "into the heads of clauses" or something like that.  This change doesn't
> cover the case of introducing them in calls in the body.

Done.

>> Index: compiler/hlds_args.m
>> ===================================================================
>> RCS file: compiler/hlds_args.m
>> diff -N compiler/hlds_args.m
>> --- /dev/null	1 Jan 1970 00:00:00 -0000
>> +++ compiler/hlds_args.m	18 Dec 2006 05:31:52 -0000
>> @@ -0,0 +1,326 @@
>> +%-----------------------------------------------------------------------------%
>> +% vim: ft=mercury ts=4 sw=4 et
>> +%-----------------------------------------------------------------------------%
>> +% Copyright (C) 2006 The University of Melbourne.
>> +% This file may only be copied under the terms of the GNU General
>> +% Public License - see the file COPYING in the Mercury distribution.
>> +%-----------------------------------------------------------------------------%
>> +%
>> +% File: hlds_args.m.
>> +% Main authors: juliensf.
>> +%
>> +% This module defines the part of the HLDS that deals with procedure and
>> call
>> +% site arguments.  (See comments at the head of polymorphism.m for further
>> +% details.)
>
> I think you should move the relevant comments here, starting from "The
> argument passing convention is..."
>
> You may need to fix some references elsewhere in polymorphism.m.

I will move the documentation eventually, but not as part of this change.

>> +%-----------------------------------------------------------------------------%
>> +
>> +:- module hlds.hlds_args.
>> +:- interface.
>> +
>> +:- import_module mdbcomp.prim_data.
>> +
>> +:- import_module list.
>> +:- import_module map.
>> +:- import_module maybe.
>> +:- import_module set.
>> +
>> +%-----------------------------------------------------------------------------%
>> +
>> +	% This type represents the arguments to a predicate symbol, or the
>> +	% arguments and result of a function symbol.  The arguments may be
>> +	% variables, types, modes, etc, depending on the context.
>> +	%
>> +	% Rather than keep all arguments in a single list, we retain
>> +	% information about the origin of each argument (such as whether
>> +	% it was introduced by polymorphism.m to hold a type_info).  This
>> +	% simplifies the processing in polymorphism.m, and also abstracts
>> +	% away the specific calling convention that we use.
>> +    %
>> +:- type arg_vector(T).
>
> Since it applies to procedure arguments, why not call it proc_args?  If we
> later add a similar data structure for data constructors, which also have
> an argument convention, what will it be called?
>
> (I think I asked you this in person, but I can't remember the answer.)

There are already a lot of names in the compiler that have either "proc" 
or "args" in them - I just found it an awkward name to work with.  How
about something like proc_arg_vector, then the version for data constructors
can be ctor_arg_vector?

>> +%-----------------------------------------------------------------------------%
>> +%
>> +% Utility predicates that operate on arg_vectors
>> +%
>> +
>> +    % Return a list of the items in a arg_vector.  The order of the
>> +    % list corresponds to that required by the calling conventions.
>> +    % See comments at the head of polymorphism.m. for details.  If the
>> +    % arg_vector represents a function then the last element in the list
>
> s/represents/is for/

Done.

>> +    % will correspond to the function return value.
>
> That last sentence is really part of the calling convention, and can be
> omitted.  It should be added in the documentation above, though.


There's no harm in repeating it here.

>> +    %
>> +:- func arg_vector_to_list(arg_vector(T)) = list(T).
>> +
>> +    % Return the set of items in an arg_vector.
>> +    %
>> +:- func arg_vector_to_set(arg_vector(T)) = set(T).
>> +
>> +    % Apply a renaming to an arg_vector.
>> +    % Useful for renaming variables etc.
>> +    %
>> +:- pred apply_renaming_to_arg_vector(map(T, T)::in,
>> +    arg_vector(T)::in, arg_vector(T)::out) is det.
>> +
>> +    % partition_arg_vector_by_origin(Vec, PolyArgs, NonPolyArgs):
>> +    %
>> +    % Partition the argument vector into two lists depending on whether
>> +    % something was introduced by the polymorphism transformation or not.
>> +    %
>> +:- pred partition_arg_vector_by_origin(arg_vector(T)::in, list(T)::out,
>> +    list(T)::out) is det.
>
> partition_arg_vector_poly_args?

I'll use this one.

> split_arg_vector_poly_args?

>> +%-----------------------------------------------------------------------------%
>> +%-----------------------------------------------------------------------------%
>> +
>> +:- implementation.
>> +
>> +:- import_module libs.compiler_util.
>> +:- import_module parse_tree.prog_util.
>> +:- import_module parse_tree.prog_type.
>> +    % Require for apply_partial_map_to_list
>> +    % XXX that should really live in a different module.
>
> In map.m, I'd suggest.
>

Okay, but as a separate change.

>> +%-----------------------------------------------------------------------------%
>> +
>> +    % The first six fields are set by the polymorphism pass.
>> +    %
>> +:- type arg_vector(T)
>> +    --->    arg_vector(
>> +                av_instance_type_infos :: list(T),
>> +                % Type_infos for the unconstrained type variables in an
>> +                % instance declaration.
>
> ...in the order that they first appear in the instance arguments.

Added.

>> +arg_vector_init(PredOrFunc, Args0) = ArgVec :-
>> +    (
>> +        PredOrFunc = predicate,
>> +        Args = Args0,
>> +        MaybeRetVal = no
>> +    ;
>> +        PredOrFunc = function,
>> +        pred_args_to_func_args(Args0, Args, RetVal),
>
> Why not just use list.det_split_last?  The pred args vs func args abstraction
> is not needed here, particularly since the comment for arg_vector_init
> states that the function result should be last.

It now uses list.det_split_last.

> If this data structure is intended to be used throughout the compiler, then
> pred_args_to_func_args should eventually become obsolete.
>
>> Index: compiler/polymorphism.m
>> ===================================================================
>> RCS file:
>> /home/mercury/mercury1/repository/mercury/compiler/polymorphism.m,v
>> retrieving revision 1.310
>> diff -u -r1.310 polymorphism.m
>> --- compiler/polymorphism.m	7 Dec 2006 15:31:12 -0000	1.310
>> +++ compiler/polymorphism.m	15 Dec 2006 03:51:58 -0000
>> @@ -462,19 +463,10 @@
>>      map.lookup(PredTable0, PredId, PredInfo0),
>>      pred_info_clauses_info(PredInfo0, ClausesInfo0),
>>      clauses_info_get_vartypes(ClausesInfo0, VarTypes0),
>> -    clauses_info_get_headvars(ClausesInfo0, HeadVars),
>> +    clauses_info_get_headvars(ClausesInfo0, HeadVarVec),
>
> Since the field is still called "headvars", I'd keep that for the variable
> names as well.  Use HeadVarList in places where a list is still used.

Okay.

Cheers,
Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------