[m-rev.] for review: restructuring parse tree items

[Julien Fischer]

On Tue, 12 Feb 2008, Zoltan Somogyi wrote:

> A first step towards rationalizing the parse tree representation of the
> program. This step moves the information specific to each kind of item
> into a structure specific to that kind of item.
>
> In the short term, this allows us to express some old invisible invariants
> as types. For example, we used to store general items in method definitions;
> we now store clause-specific data there. This allows us to simplify some code
> and eliminate some old "can't fail" tests.
>
> In the longer term, this change will allow us to replace the old list of items
> representation of the parse tree with a more structured representation,
> which aggregates each kind of item differently. For example, we could
> keep clause items in a list, but map module imports to the contexts
> of their :- import_module items, which would allow us to detect duplicate
> imports. We could also change the current three pass structure of the
> parse tree to HLDS conversion step, where each pass processes *all* items,
> to a much more flexible structure where each pass processes processes

Delete one of the "processes".

> only what it needs to process, new passes could be added much more simply,
> and in fact the whole notion of a "pass" could be eliminated.
>
> In a bunch of places, factor out some common code.
>
> compiler/prog_item.m:
> 	Make the change to the item type as above.
>
> 	Rename the old item_pred_or_func as item_pred_decl (it already had
> 	a field to indicate predicate or function) and item_pred_or_func_mode
> 	as item_mode_decl. These names are much more consistent with the
> 	other item names.
>
> 	Eliminate the item_and_context type by moving the context into
> 	the items themselves. In code that cares about contexts, this
> 	makes it easier to match up each item with its context. In code
> 	that doesn't care about contexts, this avoids the extra code
> 	that would be required to discard the item_and_context wrapper.
>
> compiler/prog_data.m:
> 	Store item_clause_infos instead of items in method definitions.
>
> compiler/prog_io.m:
> compiler/prog_io_dcg.m:
> compiler/prog_io_pragma.m:
> compiler/prog_io_typeclass.m:
> 	Construct the new item structure when creating the parse tree.
>
> 	Instead of constructing items and later attaching the context to them,
> 	pass the context down, since we now need to include them in items.
>
> 	Some old code was assuming that term.variables had no contexts;
> 	update such code.
>
> 	In prog_io_pragma.m, replace a single predicate that parsed all kinds
> 	of pragmas, which spanned more than one thousand lines and whose
> 	clauses had been interspersed with the clauses of other predicates,
> 	with a predicate whose only job is to select which of a bunch of
> 	pragma-type-specific parse predicates to invoke. Each of these
> 	pragma-type-specific parse predicates corresponds to one of the
> 	clauses of the old predicate. In that form, the predicates can be
> 	declared det, even though the predicate as a whole is semidet
> 	(since not all pragma names are valid). This actually exposed
> 	an old bug; the case MaybeAttributes = error1(_) was not handled
> 	in foreign_export_enum pragmas.
>
> 	To make the diff easier to check, I left the predicates in the
> 	original order of the clauses, even though that order does not
> 	make sense (it does not group related pragmas together). I did
> 	leave an XXX comment about this. The matter will be addressed
> 	in a later diff. (A similar problem occurs in some of the other
> 	modules in which I broke up very large predicates.)
>
> compiler/prog_io_util.m:
> 	Remove some stuff that the new item structure makes unnecessary.
>
> compiler/make_hlds_passes.m:
> compiler/add_class.m:
> compiler/add_mode.m:
> compiler/add_pragma.m:
> compiler/add_solver.m:
> 	Conform to the new item structure when converting it to HLDS.
> 	Break up excessively large predicates.
>
> compiler/prog_foreign.m:
> 	Provide a function to return all supported foreign languages,
> 	instead of requiring callers to call solutions to compute this list.
>
> compiler/mercury_to_mercury.m:
> 	Print out the new item structure.
> 	Break up excessively large predicates.
> 	Rename some predicates to avoid name collisions.
>
> compiler/equiv_type.m:
> compiler/hlds_module.m:
> compiler/intermod.m:
> compiler/make.module_dep_file.m:
> compiler/mercury_compile.m:
> compiler/module_qual.m:
> compiler/modules.m:
> compiler/prog_mutable.m:
> compiler/recompilation.check.m:
> compiler/recompilation.version.m:
> compiler/state_var.m:
> compiler/trans_opt.m:
> 	Operate on the new item structure. Factor out code (usually tests)
> 	where the new item structure makes this possible and desirable.
> 	Turn if-then-elses into switches where this is desirable.
> 	Build up large terms from named pieces instead of all at once.
> 	Break up excessively large predicates.
>
> 	In equiv_type.m, rename a predicate to clarify its function,
> 	and add an XXX about a possible improvement in abstraction.
>
> 	In modules.m, simplify the interface between some predicates
> 	and their callers, turn some predicates into functions, and
> 	make some code return error specifications instead of doing
> 	raw printing of error messages. Note that this module still
> 	has plenty of scope for improvement (I marked some with XXXs),
> 	but that is for a later date.
>
> 	In some cases, mark potential bugs with XXXs.

I think we should consider adding something to compiler/notes 
that more precisely specifies how the system of interface files
should work.  (If others feel that this would be worthwhile, I will
make a start on it.)

> compiler/equiv_type_hlds.m:
> 	Conform to the change in equiv_type.m.
>
> library/term.m:
> compiler/recompilation.check.m:
> 	Move the function for getting the context out of a term from
> 	recompilation.check.m to term.m, so it can be used from other modules.
> 	(Also, adding such a function to the standard library is long overdue.)

Please add an entry to the NEWS file for the addition to the term
module.

...

> Index: compiler/prog_item.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/compiler/prog_item.m,v
> retrieving revision 1.30
> diff -u -b -r1.30 prog_item.m
> --- compiler/prog_item.m	5 Dec 2007 05:07:37 -0000	1.30
> +++ compiler/prog_item.m	7 Feb 2008 08:18:51 -0000

...

>
> +:- type item_mutable_info
> +    --->    item_mutable_info(
>             % :- mutable(var_name, type, inst, value, attrs).
> -    ;       item_mutable(
>                 mut_name                        :: string,
>                 mut_type                        :: mer_type,
>                 mut_init_value                  :: prog_term,
>                 mut_inst                        :: mer_inst,
>                 mut_attrs                       :: mutable_var_attributes,
> -                mut_varset                      :: prog_varset
> -            )
> +                mut_varset                      :: prog_varset,
> +                mut_context                     :: prog_context
> +            ).
>
> -            % Used for items that should be ignored (for the
> -            % purposes of backwards compatibility etc).
> -    ;       item_nothing(
> -                nothing_maybe_warning           :: maybe(item_warning)
> +:- type item_nothing_info
> +    --->    item_nothing_info(
> +                % Used for items that should be ignored (for purposes of
> +                % backwards compatibility etc).
> +                nothing_maybe_warning           :: maybe(item_warning),
> +                nothing_context                 :: prog_context
>             ).
>
> -:- inst item_mutable
> -    --->    item_mutable(ground, ground, ground, ground, ground, ground).
> +:- func get_item_context(item) = prog_context.
> +
> +% XXX
> +% :- inst item_mutable
> +%     --->    item_mutable(ground, ground, ground, ground, ground, ground).

Delete that commented out inst - it shouldn't be needed any more.

You can go ahead and commit this (the XXXs you have identified should be
dealt with separately.)

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