[m-dev.] for review: centralizing variable names in the debugger

[Mark Anthony BROWN]

Zoltan Somogyi writes:
> For review by Mark.
> 
> Store the offsets leading to the names of variables in a central location.
> Previously, each label layout included offsets for the variables live at that
> label; now we store the offsets of variables in the proc layout. The new
> data structure will soon be required by the declarative debugger for
> displaying contours through representations of HLDS goals.
> 
> compiler/code_gen.m:
> 	Preserve the varset of every procedure for use by stack_layout.m.
> 
> compiler/continuation_info.m:
> 	Add a field to the cell from which proc layouts are derived to hold
> 	the varset.
> 
> compiler/stack_layout.m:
> 	Delete the offsets from label layouts and add them to proc layouts.
> 	The var name offsets array will always contain meaningful offsets
> 	for every variable that is live at a label with a label layout; with
> 	--trace-decl, it will also contain meaningful offsets for all the
> 	other variables in the procedure body.
> 
> runtime/mercury_stack_layout.h:
> 	Reflect the change in the types of the label and proc layout
> 	structures.
> 
> trace/mercury_trace.c:
> trace/mercury_trace_vars.c:
> 	Look up variable names using the new location of the offsets leading to
> 	them.
> 
> Zoltan.
> 

This change is fine.  I have just a few comments:


> Index: compiler/stack_layout.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/stack_layout.m,v
> retrieving revision 1.49
> diff -u -r1.49 stack_layout.m
> --- compiler/stack_layout.m	2000/09/04 22:33:50	1.49
> +++ compiler/stack_layout.m	2000/09/11 03:33:41

> @@ -1121,17 +1192,17 @@
>  			rval,	% Rval describing the type of a live value.
>  			llds_type, % The llds type of the rval describing the
>  				% type.
> -			rval,	% Rval describing the variable number of a
> -				% live value. Always of llds uint_least16.
> +			rval	% Rval describing the variable number of a
> +				% live value. Always of llds type uint_least16.
>  				% Contains zero if the live value is not
>  				% a variable. Contains the hightest possible
>  				% uint_least16 value if the variable number
>  				% does not fit in 16 bits.
> -			rval	% Rval describing the variable name of a
> -				% live value. Always of llds uint_least16.
> -				% Contains zero if the live value is not
> -				% a variable, or if it is a variable with
> -				% no name.
> +	%		rval	% Rval describing the variable name of a
> +	%			% live value. Always of llds type uint_least16.
> +	%			% Contains zero if the live value is not
> +	%			% a variable, or if it is a variable with
> +	%			% no name.
>  		).

Why not remove this field entirely?

>  
>  	% Construct a vector of (locn, live_value_type) pairs,
> @@ -1142,7 +1213,7 @@
>  	stack_layout_info::in, stack_layout_info::out) is det.
>  
>  stack_layout__construct_liveval_arrays(VarInfos, LengthRval,
> -		TypeLocnVector, NameVector) -->
> +		TypeLocnVector, NumVector) -->
>  	{ int__pow(2, stack_layout__short_count_bits, BytesLimit) },
>  	stack_layout__construct_liveval_array_infos(VarInfos,
>  		0, BytesLimit, IntArrayInfo, ByteArrayInfo),
> @@ -1156,21 +1227,21 @@
>  	{ LengthRval = const(int_const(EncodedLength)) },
>  
>  	{ SelectLocns = lambda([ArrayInfo::in, MaybeLocnRval::out] is det, (
> -		ArrayInfo = live_array_info(LocnRval, _, _, _, _),
> +		ArrayInfo = live_array_info(LocnRval, _, _, _),
>  		MaybeLocnRval = yes(LocnRval)
>  	)) },
>  	{ SelectTypes = lambda([ArrayInfo::in, MaybeTypeRval::out] is det, (
> -		ArrayInfo = live_array_info(_, TypeRval, _, _, _),
> +		ArrayInfo = live_array_info(_, TypeRval, _, _),
>  		MaybeTypeRval = yes(TypeRval)
>  	)) },
>  	{ SelectTypeTypes = lambda([ArrayInfo::in, CountTypeType::out] is det,(
> -		ArrayInfo = live_array_info(_, _, TypeType, _, _),
> +		ArrayInfo = live_array_info(_, _, TypeType, _),
>  		CountTypeType = 1 - yes(TypeType)
>  	)) },
> -	{ AddRevNumsNames = lambda([ArrayInfo::in, NumNameRvals0::in,
> -			NumNameRvals::out] is det, (
> -		ArrayInfo = live_array_info(_, _, _, NumRval, NameRval),
> -		NumNameRvals = [yes(NameRval), yes(NumRval) | NumNameRvals0]
> +	{ AddRevNums = lambda([ArrayInfo::in, NumRvals0::in,
> +			NumRvals::out] is det, (
> +		ArrayInfo = live_array_info(_, _, _, NumRval),
> +		NumRvals = [yes(NumRval) | NumRvals0]
>  	)) },

Is there still a reason to stick to the old syntax for lambda expressions?


> Index: runtime/mercury_stack_layout.h
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_stack_layout.h,v
> retrieving revision 1.37
> diff -u -r1.37 mercury_stack_layout.h
> --- runtime/mercury_stack_layout.h	2000/08/03 06:18:54	1.37
> +++ runtime/mercury_stack_layout.h	2000/09/11 03:41:53
> @@ -248,8 +248,8 @@
>  ** The last three fields are meaningful only if the MR_has_valid_var_count
>  ** macro returns true.
>  **
> -** The names array pointer may be NULL, in which case no information about
> -** variable names is available.
> +** The var nums array pointer may be NULL, in which case no information about
> +** variable numbers is available.
>  **
>  ** The type parameters array may also be NULL, but this means that there are
>  ** no type parameters in the types of the variables live at this point.
> @@ -258,9 +258,9 @@
>  */
>  
>  typedef	struct MR_Stack_Layout_Vars_Struct {
> -	MR_Integer			MR_slvs_var_count;
> +	MR_Integer		MR_slvs_var_count;
>  	void			*MR_slvs_locns_types;
> -	MR_Var_Name		*MR_slvs_names;
> +	MR_uint_least16_t	*MR_slvs_var_nums;
>  	MR_Type_Param_Locns	*MR_slvs_tvars;
>  } MR_Stack_Layout_Vars;
>  

I think there are no other places where MR_Var_Name is used, so you could
delete its definition from this file.

Cheers,
Mark.

--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------