[m-rev.] for (possibly post-commit) review: simplify the creation of .int files

[Julien Fischer]

On Fri, 15 Feb 2019, Zoltan Somogyi wrote:

> This is the major part of step 1d of the plan. I would like to start
> working on the minor part, simplifying the code that creates .int2
> files from .int files, so this diff keeps the old, convoluted code for
> creating .int files, and checks that the new code generates the exact
> same output.
>
> Given that fact, and given that this diff has passed a hlc bootcheck cleanly,
> are people ok with me committing this diff now? And does anyone
> want to put their hand up to say they will review it?


> Construct .int files much more directly.
> 
> The predicates in write_module_interface_files.m that construct .int files
> have traditionally done so in a fairly convoluted manner. This diff adds
> a way to construct them much more directly.

...

> diff --git a/compiler/comp_unit_interface.m b/compiler/comp_unit_interface.m
> index 31ef4d6..d27fe1b 100644
> --- a/compiler/comp_unit_interface.m
> +++ b/compiler/comp_unit_interface.m

...

> +:- pred accumulate_abs_imp_exported_type_lhs_2(type_defn_map::in,
> +    type_defn_map::in, type_ctor::in, item_type_defn_info::in,
> +    set(type_ctor)::in, set(type_ctor)::out,
> +    set(type_ctor)::in, set(type_ctor)::out,
> +    set(type_ctor)::in, set(type_ctor)::out) is det.
> +
> +accumulate_abs_imp_exported_type_lhs_2(IntTypesMap, BothTypesMap,
> +        TypeCtor, ImpItemTypeDefnInfo, !AbsExpEqvLhsTypeCtors,
> +        !AbsExpEnumTypeCtors, !DummyTypeCtors) :-
> +    ImpItemTypeDefnInfo = item_type_defn_info(_, _, ImpTypeDefn, _, _, _),
> +    (
> +        ImpTypeDefn = parse_tree_eqv_type(_),
> +        ( if map.search(IntTypesMap, TypeCtor, _) then
> +            set.insert(TypeCtor, !AbsExpEqvLhsTypeCtors)
> +        else
> +            true
> +        )
> +    ;
> +        ImpTypeDefn = parse_tree_foreign_type(_),
> +        ( if map.search(IntTypesMap, TypeCtor, _) then
> +            % XXX ITEM_LIST This looks like a lost opportunity to me,
> +            % because the only foreign types that *need* the same treatment
> +            % as equivalence types are foreign types that are bigger than
> +            % one word in size. These should be extremely rare, so it should
> +            % be ok to require programmers to signal them with a specific
> +            % attribute on the foreign type definition.

We do not necessarily know if a foreign type is going to be larger than a word
in size, since that can vary depending on the target platform  For example:

    :- pragma foreign_type("C", foo, "int64_t").

I think the most useful additional annotation we could have on foreign types,
would be one that allowed their size (or at least an upper bound on their size)
to be specified.

...

> +%---------------------%
> +
> +    % Give a type constructor's type definitions from the implementation

s/Give/Given/

> +    % section, as recorded in ImpTypesMap, include their abstract versions
> +    % in !ImpTypeDefnItems, the list of type definition items scheduled to be
> +    % added back to the implementation section, *provided* that
> +    %
> +    % - the type constructor is in NeededTypeCtors, and
> +    %
> +    % - *either* the type has no declaration or definition in the interface,
> +    %   *or* at least one of the type definitions in the implementation section
> +    %   contains more information than a pure abstract type declaration
> +    %   (such as may be found in the interface section) would.
> +    %
> +    % By "pure abstract" type declarations, we mean abstract type
> +    % declarations that give no further implementation. This means that
> +    % `type_is_abstract_enum' declarations are not *pure* abstract.
> +    % XXX ITEM_LIST I (zs) believe that the intention behind this proviso
> +    % was to allow items representing the following scenario to be left
> +    % alone:
> +    %
> +    % :- interface.
> +    % :- type t1.
> +    % ...
> +    % :- implementation.
> +    % :- type t1 where type_is_abstract_enum(...).
> +    %
> +    % XXX ITEM_LIST Just because *one* definition in the implementation has
> +    % more info than a pure abstract type declaration *should not* result in
> +    % us adding back to the implementation section any other type definitions
> +    % that *do* represent nothing more than a pure abstract type declaration.
> +    % Note that this distinction should matter only for types whose set of
> +    % definitions are erroneous, such a type that is defined both as
> +    % an equivalence type and as a du type.
> +    % 
> +:- pred maybe_add_maybe_abstract_type_defn_items(
> +    type_defn_map::in, type_defn_map::in, set(type_ctor)::in,
> +    type_ctor::in, list(item_type_defn_info)::in,
> +    list(item)::in, list(item)::out) is det.

The change looks fine otherwise.

Julien.