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

Fergus Henderson fjh at cs.mu.OZ.AU
Mon Oct 30 18:41:32 AEDT 2000


On 30-Oct-2000, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> For review by anyone.
> 
> This is part 2 of a change that provides a register of all the types defined
> in the program.
> 
> util/mkinit.c:
> 	After part 1 of this change, each compiler-generated module has three
> 	initialization functions: the old one (to register label addresses
> 	etc), one to register type_ctor_infos, and one to register module
> 	layouts for the debugger. However, only the first was invoked from
> 	the mkinit generated <mainmodule>_init.c file.
> 
> 	This change invokes the other two as well. One complication is that
> 	hand-written "modules" do not have the two new kinds of initialization
> 	functions, so only their first initialization function should be
> 	called. We do this by requiring those "modules" to have one of two
> 	specific forms: an initial prefix of either sys_init or
> 	mercury_sys_init.
....
> library/array.m:
> library/builtin.m:
> library/private_builtin.m:
> library/std_util.m:
> 	Add code to initialization functions to register the type_ctor_infos
> 	of hand-defined types.
> 
> 	Note that this code is in the usual initialization function, the one
> 	called by do_init_modules(). Putting this code in a separate
> 	initialization function that is called by do_init_modules_type_tables()
> 	would require complicating mkinit.c considerably.

I must be missing something...
what extra complication would be required in mkinit.c?

The log message above suggests that extra complication is
required in mkinit.c because the code is in a single function.

> +++ util/mkinit.c	2000/10/14 10:35:50
> +static int
> +output_sub_init_functions(const char *suffix, bool wrap_func_in_ifdef,
> +	bool only_full_module)
>  {
>  	int filenum;
> +	int	filebunches;
> +	int	cur_bunch;
...
> +	filebunches = output_sub_init_functions("",
> +			! need_initialization_code, FALSE);
> +	output_main_init_function("",
> +			! need_initialization_code, filebunches);
> +
> +	filebunches = output_sub_init_functions("_type_tables", FALSE, TRUE);
> +	output_main_init_function("_type_tables", FALSE, filebunches);
> +
> +	filebunches = output_sub_init_functions("_debugger", TRUE, TRUE);
> +	output_main_init_function("_debugger", TRUE, filebunches);

What do those variables `filebunches' and `cur_bunch' represent?
Some comments and/or better variable names would help here.

If `cur_bunch' represents the number of function calls output so far
in the current function, then I think `num_calls_in_cur_func'
would be a better name.

I think it would be clearer if `filebunches' was named
`num_<something>' (throughout).  My first thought on reading

	filebunches = output_sub_init_functions("",

was that filebunches was probably some collection (e.g. a linked
list) of all of the "file bunches" (whatever they were).
Using a `num_' prefix for variables which hold a count
rather than a collection improves readability, IMHO.

> +#if 0
> +	if (need_tracing)  {
> +		files = (char **) checked_malloc((num_files + 1)
> +				* sizeof(char *));
> +		for (i = 0; i < num_files; i++) {
> +			files[i] = argv[optind + i];
> +		}
> +		strcpy(files[num_files], "mer_browser.init");
> +		num_files++;
> +	} else {
>  	files = argv + optind;
> +	}
> +#endif

What's that code inside `#if 0' for?

Apart from those issues, this change looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
                                    |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------



More information about the developers mailing list