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

[Tyson Dowd]

I'm not sure whether you are aware of io__write_list or have just chosen
not to use it.  Since I think it will improve the readability of the
code quite a bit to use it, I've pointed out where it would be useful.

On 25-Feb-2000, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Index: compiler/rtti.m
> ===================================================================
> RCS file: rtti.m
> diff -N rtti.m
> --- /dev/null	Thu Sep  2 15:00:04 1999
> +++ rtti.m	Tue Feb 22 14:12:39 2000
> @@ -0,0 +1,560 @@
> +%-----------------------------------------------------------------------------%
> +% Copyright (C) 2000 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.
> +%-----------------------------------------------------------------------------%
> +%
> +% Definitions of data structures for representing run-time type information
> +% within the compiler.
> +%
> +% Eventually, this module will be independent of whether we are compiling
> +% to LLDS or MLDS. For the time being, it depends on LLDS.
> +%
> +% Author: zs.
> +
> +%-----------------------------------------------------------------------------%
> +
> +:- module rtti.
> +
> +:- interface.
> +
> +:- import_module llds, prog_data.
> +:- import_module bool, list, std_util.
> +
> +:- type sectag_locn
> +	--->	sectag_none
> +	;	sectag_local
> +	;	sectag_remote.
> +
> +:- type equality_axioms
> +	--->	standard
> +	;	user_defined.

These types need one line explanations.

> +
> +:- type type_ctor_layout_info
> +	--->	enum_layout(
> +			rtti_name
> +		)
> +	;	notag_layout(
> +			rtti_name
> +		)
> +	;	du_layout(
> +			rtti_name
> +		)
> +	;	equiv_layout(
> +			rval
> +		)
> +	;	no_layout.
> +
> +:- type type_ctor_functors_info
> +	--->	enum_functors(
> +			rtti_name
> +		)
> +	;	notag_functors(
> +			rtti_name
> +		)
> +	;	du_functors(
> +			rtti_name
> +		)
> +	;	no_functors.

These types need explanations too.

> +
> +:- type exist_typeinfo_locn
> +	--->	plain_typeinfo(
> +			int			% The typeinfo is stored
> +						% directly in the cell, at this
> +						% offset.
> +		)
> +	;	typeinfo_in_tci(
> +			int,			% The typeinfo is stored
> +						% indirectly in the typeclass
> +						% info stored at this offset
> +						% in the cell.
> +			int			% To find the typeinfo inside
> +						% the typeclass info structure,
> +						% give this integer to the
> +						% MR_typeclass_info_type_info
> +						% macro.
> +		).
> +
> +:- type rtti_type_id
> +	--->	rtti_type_id(
> +			module_name,		% module name
> +			string,			% type ctor's name
> +			arity			% type ctor's arity
> +		).
> +
> +	% Global data generated by the compiler. Usually readonly,
> +	% with one exception: data containing code addresses must
> +	% be initialized.
> +:- type rtti_data

s/must/might
s/initialized/initialized at runtime in grades that don't support static
code initializers/

Also you should mention that this data corresponds to structures in 
mercury_type_info.h (so they know where to look for the MR_* C types).

> +	--->	exist_locns(
> +			rtti_type_id,		% identifies the type
> +			int,			% identifies functor in type
> +
> +			% The remaining argument of this function symbol
> +			% corresponds to an array of MR_ExistTypeInfoLocns.
> +
> +			list(exist_typeinfo_locn)
> +		)
> +	;	exist_info(
> +			rtti_type_id,		% identifies the type
> +			int,			% identifies functor in type
> +
> +			% The remaining arguments of this function symbol
> +			% correspond to the MR_DuExistInfo C type.
> +
> +			int,			% number of plain typeinfos
> +			int,			% number of typeinfos in tcis
> +			int,			% number of tcis
> +			rtti_name		% table of typeinfo locations
> +		)
> +	;	field_names(
> +			rtti_type_id,		% identifies the type
> +			int,			% identifies functor in type
> +
> +			list(maybe(string))	% gives the field names
> +		)
> +	;	enum_functor_desc(
> +			rtti_type_id,		% identifies the type
> +
> +			% The remaining arguments of this function symbol
> +			% correspond one-to-one to the fields of
> +			% MR_EnumFunctorDesc.

s/MR_EnumFunctorDesc/the MR_EnumFunctorDesc C type/

> +
> +			string,			% functor name
> +			int			% ordinal number of functor
> +						% (also its value)
> +		)
> +	;	notag_functor_desc(
> +			rtti_type_id,		% identifies the type
> +
> +			% The remaining arguments of this function symbol
> +			% correspond one-to-one to the fields of
> +			% MR_NotagFunctorDesc.

s/MR_NotagFunctorDesc/the MR_NotagFunctorDesc C type/

> +
> +			string,			% functor name
> +			rval			% pseudo typeinfo of argument
> +		)
> +	;	du_functor_desc(
> +			rtti_type_id,		% identifies the type
> +
> +			% The remaining arguments of this function symbol
> +			% correspond one-to-one to the fields of
> +			% MR_DuFunctorDesc.

s/MR_DuFunctorDesc/the MR_DuFunctorDesc C type/

> +:- pred rtti__make_exist_info_name(rtti_type_id::in, int::in,
> +	string::out) is det.
> +
> +	% Create a C variable name for the array listing the names
> +	% of the fields of a function symbol.

Why are these (and the other similar functions) exported?  They don't
appear to be used outside this module.

> +
> +:- pred rtti_addr_to_string(rtti_type_id::in, rtti_name::in, string::out)
> +	is det.

rtti_addr should be rtti__addr.

> +
> +:- pred rtti__sectag_locn_to_string(sectag_locn::in, string::out) is det.
> +
> +:- pred rtti__type_ctor_rep_to_string(type_ctor_rep::in, string::out) is det.
> +
> +:- pred rtti__name_would_include_code_address(rtti_name::in, bool::out) is det.

These predicates need comments.

> +
> +:- implementation.
> +
> +:- import_module llds_out.
> +:- import_module string.
> +
> +rtti__make_exist_locns_name(RttiTypeId, Ordinal, Str) :-
> +	RttiTypeId = rtti_type_id(ModuleName0, TypeName0, TypeArity),
> +	llds_out__sym_name_mangle(ModuleName0, ModuleName),
> +	llds_out__name_mangle(TypeName0, TypeName),
> +	string__int_to_string(TypeArity, A_str),
> +	string__int_to_string(Ordinal, O_str),
> +        string__append_list([ModuleName, "__exist_locns_", TypeName,
> +		"_", A_str, "_", O_str], Str).

This cut-and-paste section of code is a bit of a maintenance nightmare.
There's a lot of common code that could be factored, and a couple of
conditionals would handle the different cases.  There doesn't seem to be
any reason to have each of these cases as a different predicate (if
there is, I can't seem to find it in your diff).

> +:- pred output_maybe_rtti_addrs(rtti_type_id::in, list(maybe(rtti_name))::in,
> +	io__state::di, io__state::uo) is det.
> +
> +output_maybe_rtti_addrs(_, []) --> [].
> +output_maybe_rtti_addrs(RttiTypeId, [MaybeRttiName | MaybeRttiNames]) -->
> +	io__write_string("\t"),
> +	(
> +		{ MaybeRttiName = yes(RttiName) },
> +		io__write_string("&"),
> +		output_rtti_addr(RttiTypeId, RttiName)
> +	;
> +		{ MaybeRttiName = no },
> +		io__write_string("NULL")
> +	),
> +	(
> +		{ MaybeRttiNames = [] },
> +		io__write_string("\n")
> +	;
> +		{ MaybeRttiNames = [_|_] },
> +		io__write_string(",\n"),
> +		output_maybe_rtti_addrs(RttiTypeId, MaybeRttiNames)
> +	).

This is a list writer.  

> +:- pred output_rtti_addrs(rtti_type_id::in, list(rtti_name)::in,
> +	io__state::di, io__state::uo) is det.
> +
> +output_rtti_addrs(_, []) --> [].
> +output_rtti_addrs(RttiTypeId, [RttiName | RttiNames]) -->
> +	io__write_string("\t&"),
> +	output_rtti_addr(RttiTypeId, RttiName),
> +	(
> +		{ RttiNames = [] },
> +		io__write_string("\n")
> +	;
> +		{ RttiNames = [_|_] },
> +		io__write_string(",\n"),
> +		output_rtti_addrs(RttiTypeId, RttiNames)
> +	).
> +

This is another list writer.  

(There are quite a few other ones in this module, by converting everything to
lists of strings and io__write_list-ing them the code could be
simplified quite a bit.  Most of the code converts to strings before
writing anyway).

> +output_rtti_addr(RttiTypeId, RttiName) -->
> +	io__write_string(mercury_data_prefix),
> +	{ rtti_addr_to_string(RttiTypeId, RttiName, Str) },
> +	io__write_string(Str).
> +
> +:- pred output_maybe_strings(list(maybe(string))::in,
> +	io__state::di, io__state::uo) is det.
> +
> +output_maybe_strings([]) -->
> +	{ error("reached empty list of maybe strings") }.
> +output_maybe_strings([MaybeName | MaybeNames]) -->
> +	io__write_string("\t"),
> +	(
> +		{ MaybeName = yes(Name) },
> +		io__write_string(""""),
> +		io__write_string(Name),
> +		io__write_string("""")
> +	;
> +		{ MaybeName = no },
> +		io__write_string("NULL")
> +	),
> +	(
> +		{ MaybeNames = [] },
> +		io__write_string("\n")
> +	;
> +		{ MaybeNames = [_|_] },
> +		io__write_string(",\n"),
> +		output_maybe_strings(MaybeNames)
> +	).

Another one.

> +
> +:- pred output_exist_locns(list(exist_typeinfo_locn)::in,
> +	io__state::di, io__state::uo) is det.
> +
> +output_exist_locns([]) -->
> +	{ error("reached empty list of exist locns") }.
> +output_exist_locns([Locn | Locns]) -->
> +	io__write_string("\t"),
> +	(
> +		{ Locn = plain_typeinfo(SlotInCell) },
> +		io__write_string("{ "),
> +		io__write_int(SlotInCell),
> +		io__write_string(", -1 }")
> +	;
> +		{ Locn = typeinfo_in_tci(SlotInCell, SlotInTci) },
> +		io__write_string("{ "),
> +		io__write_int(SlotInCell),
> +		io__write_string(", "),
> +		io__write_int(SlotInTci),
> +		io__write_string(" }")
> +	),
> +	(
> +		{ Locns = [] },
> +		io__write_string("\n")
> +	;
> +		{ Locns = [_|_] },
> +		io__write_string(",\n"),
> +		output_exist_locns(Locns)
> +	).

Another one.

Otherwise this module is fine.  I think the new type for the rtti data
structures is a big improvement.

-- 
The quantum sort: 
	while (!sorted) { do_nothing(); }
Tyson Dowd   <tyson at tyse.net>   http://tyse.net/
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------