[m-dev.] for review: cleanup of type_ctor_infos, part 0

[Tyson Dowd]

note the subject line should be part 1 not part 0

For this part I just have a few comments on style.

>  	->
> -		Refs = Refs0
> +		Refs0 = [],
> +		dead_proc_elim__maybe_add_ref(MaybeUnify,   Refs0, Refs1),
> +		dead_proc_elim__maybe_add_ref(MaybeIndex,   Refs1, Refs2),
> +		dead_proc_elim__maybe_add_ref(MaybeCompare, Refs2, Refs3),
> +		dead_proc_elim__maybe_add_ref(MaybeSolver,  Refs3, Refs4),
> +		dead_proc_elim__maybe_add_ref(MaybeInit,    Refs4, Refs5),
> +		dead_proc_elim__maybe_add_ref(MaybePretty,  Refs5, Refs6),
> +		Refs = Refs6

list__foldl would be a more concise and maintainable way to write this
kind of thing.  

> @@ -559,21 +533,22 @@
>  
>  :- type module_info
>  	--->	module(
> -			module_sub_info,
> -			predicate_table,
> -			proc_requests,
> -			special_pred_map,
> -			partial_qualifier_info,
> -			type_table,
> -			inst_table,
> -			mode_table,
> -			cons_table,
> -			class_table,
> -			instance_table,
> -			superclass_table,
> -			assertion_table,
> -			ctor_field_table,
> -			int		% cell count, passed into code_info
> +			module_sub_info :: module_sub_info,
> +			pred_table :: predicate_table,
> +			proc_requests :: proc_requests,
> +			special_pred_map :: special_pred_map,
> +			partial_qualifier_info :: partial_qualifier_info,
> +			type_table :: type_table,
> +			inst_table :: inst_table,
> +			mode_table :: mode_table,
> +			cons_table :: cons_table,
> +			class_table :: class_table,
> +			instance_table :: instance_table,
> +			superclass_table :: superclass_table,
> +			assertion_table :: assertion_table,
> +			ctor_field_table :: ctor_field_table,
> +			cell_count :: int
> +					% cell count, passed into code_info
>  					% and used to generate unique label
>  					% numbers for constant terms in the
>  					% generated C code

I'm going to suggest we adopt a coding style for named fields, where we
indent the ":: type" part much the same way we have traditionally
indented the comments for each field:

			module_sub_info		:: module_sub_info,
			pred_table 		:: predicate_table,
			proc_requests		:: proc_requests,
			special_pred_map 	:: special_pred_map,
			partial_qualifier_info	:: partial_qualifier_info,
			type_table 		:: type_table,
			inst_table 		:: inst_table,
			mode_table 		:: mode_table,
			cons_table 		:: cons_table,
			class_table 		:: class_table,
			instance_table 		:: instance_table,
			superclass_table 	:: superclass_table,
			assertion_table 	:: assertion_table,
			ctor_field_table 	:: ctor_field_table,
			cell_count 		:: int
					% cell count, passed into code_info
 					% and used to generate unique label
 					% numbers for constant terms in the
 					% generated C code

I think this is much more readable.

> +	% Output a list of maybe data addresses, with a `no' meaning NULL.
> +
> +:- pred output_maybe_data_addrs(list(maybe(data_addr))::in,
> +	io__state::di, io__state::uo) is det.
> +
> +output_maybe_data_addrs([]) --> [].
> +output_maybe_data_addrs([MaybeDataAddr | DataAddrs]) -->
> +	io__write_string("\t"),
> +	(
> +		{ MaybeDataAddr = yes(DataAddr) },
> +		output_data_addr(DataAddr)
> +	;
> +		{ MaybeDataAddr = no },
> +		io__write_string("NULL")
> +	),
> +	(
> +		{ DataAddrs = [] },
> +		io__write_string("\n")
> +	;
> +		{ DataAddrs = [_|_] },
> +		io__write_string(",\n"),
> +		output_maybe_data_addrs(DataAddrs)
> +	).

This is an odd piece of code.  I find io__write_list much easier to
read -- something like this:

output_maybe_data_addrs([]) --> [].
output_maybe_data_addrs([MaybeDataAddr | DataAddrs]) -->
	{ MaybeOutputDataAddr = (pred(M::in, di, uo) -->
		( { M = yes(D) } ->
			output_data_addr(D)
		;	
			io__write_string("NULL")
		) ) },
	io__write_string("\t"),
	io__write_list([MabyeDataAddr | DataAddrs], ",\n\t", 
		MaybeOutputDataAddr),
	io__write_string("\n").

Then the actual actions per element are well separated from the output
of the list, and you don't clutter the code with look ahead.


> +	% Output a list of data addresses.
> +
> +:- pred output_data_addrs(list(data_addr)::in, io__state::di, io__state::uo)
> +	is det.
> +
> +output_data_addrs([]) --> [].
> +output_data_addrs([DataAddr | DataAddrs]) -->
> +	io__write_string("\t"),
> +	output_data_addr(DataAddr),
> +	(
> +		{ DataAddrs = [] },
> +		io__write_string("\n")
> +	;
> +		{ DataAddrs = [_|_] },
> +		io__write_string(",\n"),
> +		output_data_addrs(DataAddrs)
> +	).

I'm pretty sure this:

output_data_addrs([]) --> [].
output_data_addrs([DataAddr | DataAddrs]) -->
	io__write_string("\t"),
	io__write_list([DataAddr | DataAddrs], ",\n\t", output_data_addr),
	io__write_string("\n").

will do the same thing.  The pattern is just like the last one.


Tyson.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------