[m-rev.] for review: parse_tree_some_int

[Julien Fischer]

Hi Zoltan,

On Wed, 28 Apr 2021, Zoltan Somogyi wrote:

> Don't use parse_tree_int in recompilation.check.m.
> 
> We would strongly prefer representing interface files using
> file-kind-specific parse trees, but smart recompilation has not yet been
> converted to use them. This diff is a start on fixing that.
> 
> compiler/prog_item.m:
>     Define a type that can represent the parse tree of any kind of interface
>     file *without* losing any file-kind-specific information.
> 
> compiler/read_modules.m:
>     Define a predicate that can read in any kind of interface file
>     and return its contents using the new representation.
> 
> compiler/recompilation.check.m:
>     Start using the above new functionality. While the old code required
>     e.g. wrapping up each type definition item in an interface file parse tree
>     in a generic item wrapper, putting it in an item list, putting that
>     into an item block, and the later traversing a list of item blocks,
>     traversing lists of items, and switching on the kind of each item.
>     Almost all of that work was wasted. The new code is much more direct,

...


> diff --git a/compiler/read_modules.m b/compiler/read_modules.m
> index 795e90a92..7c5580b08 100644
> --- a/compiler/read_modules.m
> +++ b/compiler/read_modules.ms

> @@ -224,6 +221,13 @@
>      parse_tree_int::out, list(error_spec)::out, read_module_errors::out,
>      io::di, io::uo) is det.
> 
> +:- pred read_module_some_int(globals::in, string::in,
> +    maybe_ignore_errors::in, maybe_search::in,
> +    module_name::in, int_file_kind::in, file_name::out,
> +    read_module_and_timestamps::in, maybe(timestamp)::out,
> +    parse_tree_some_int::out, list(error_spec)::out, read_module_errors::out,
> +    io::di, io::uo) is det.
> +
>      % Versions of read_module_int that read and return a non-generic
>      % parse tree.
>      %
> @@ -300,10 +304,11 @@
>      maybe_return_timestamp::in, file_name::out, maybe(timestamp)::out,
>      parse_tree_src::out, list(error_spec)::out, read_module_errors::out)
>      is semidet.
> -:- pred find_read_module_int(have_read_module_int_map::in,
> -    have_read_module_key(int_file_kind)::in,
> +
> +:- pred find_read_module_some_int(have_read_module_maps::in,
> +    module_name::in, int_file_kind::in,
>      maybe_return_timestamp::in, file_name::out, maybe(timestamp)::out,
> -    parse_tree_int::out, list(error_spec)::out, read_module_errors::out)
> +    parse_tree_some_int::out, list(error_spec)::out, read_module_errors::out)
>      is semidet.
>
>      % Versions of find_read_module_int that return a non-generic parse tree.

...

> @@ -765,14 +799,34 @@ find_read_module_src(HaveReadModuleMapSrc, ModuleName, ReturnTimestamp,
>      return_timestamp_if_needed(ReturnTimestamp, MaybeTimestamp0,
>          MaybeTimestamp).
> 
> -find_read_module_int(HaveReadModuleMapInt, Key,
> +find_read_module_some_int(HaveReadModuleMaps, ModuleName, IntFileKind,
>          ReturnTimestamp, FileName, MaybeTimestamp,
> -        ParseTreeInt, Specs, Errors) :-
> -    map.search(HaveReadModuleMapInt, Key, HaveReadModule),
> -    HaveReadModule = have_successfully_read_module(FileName, MaybeTimestamp0,
> -        ParseTreeInt, Specs, Errors),
> -    return_timestamp_if_needed(ReturnTimestamp, MaybeTimestamp0,
> -        MaybeTimestamp).
> +        ParseTreeSomeInt, Specs, Errors) :-
> +    (
> +        IntFileKind = ifk_int0,
> +        find_read_module_int0(HaveReadModuleMaps ^ hrmm_int0, ModuleName,
> +            ReturnTimestamp, FileName, MaybeTimestamp,
> +            ParseTreeInt0, Specs, Errors),
> +        ParseTreeSomeInt = parse_tree_some_int0(ParseTreeInt0)
> +    ;
> +        IntFileKind = ifk_int1,
> +        find_read_module_int1(HaveReadModuleMaps ^ hrmm_int1, ModuleName,
> +            ReturnTimestamp, FileName, MaybeTimestamp,
> +            ParseTreeInt1, Specs, Errors),
> +        ParseTreeSomeInt = parse_tree_some_int1(ParseTreeInt1)
> +    ;
> +        IntFileKind = ifk_int2,
> +        find_read_module_int2(HaveReadModuleMaps ^ hrmm_int2, ModuleName,
> +            ReturnTimestamp, FileName, MaybeTimestamp,
> +            ParseTreeInt2, Specs, Errors),
> +        ParseTreeSomeInt = parse_tree_some_int2(ParseTreeInt2)
> +    ;
> +        IntFileKind = ifk_int3,
> +        find_read_module_int3(HaveReadModuleMaps ^ hrmm_int3, ModuleName,
> +            ReturnTimestamp, FileName, MaybeTimestamp,
> +            ParseTreeInt3, Specs, Errors),
> +        ParseTreeSomeInt = parse_tree_some_int3(ParseTreeInt3)
> +    ).

Add a require_complete_switch scope to the switch on IntFileKind.

...

> diff --git a/compiler/recompilation.check.m b/compiler/recompilation.check.m
> index 2944e63ae..da4bb0458 100644
> --- a/compiler/recompilation.check.m
> +++ b/compiler/recompilation.check.m

...

> @@ -2132,6 +2170,167 @@ record_recompilation_reason(Reason, MaybeStoppingReason, !Info) :-
>          MaybeStoppingReason = yes(Reason)
>      ).
> 
> +%---------------------------------------------------------------------------%
> +
> +:- pred get_version_numbers_from_parse_tree_some_int(parse_tree_some_int::in,
> +    version_numbers::out) is semidet.
> +
> +get_version_numbers_from_parse_tree_some_int(ParseTreeSomeInt,
> +        VersionNumbers) :-
> +    (
> +        ParseTreeSomeInt = parse_tree_some_int0(ParseTreeInt0),
> +        MaybeVersionNumbers = ParseTreeInt0 ^ pti0_maybe_version_numbers
> +    ;
> +        ParseTreeSomeInt = parse_tree_some_int1(ParseTreeInt1),
> +        MaybeVersionNumbers = ParseTreeInt1 ^ pti1_maybe_version_numbers
> +    ;
> +        ParseTreeSomeInt = parse_tree_some_int2(ParseTreeInt2),
> +        MaybeVersionNumbers = ParseTreeInt2 ^ pti2_maybe_version_numbers
> +    ;
> +        ParseTreeSomeInt = parse_tree_some_int3(_ParseTreeInt3),
> +        % .int3 files never contain version numbers.
> +        fail
> +    ),
> +    MaybeVersionNumbers = version_numbers(VersionNumbers).


Add a require_complete_switch scope for the switch on ParseTreeSomeInt.

> +%---------------------------------------------------------------------------%
> +
> +:- type ambiguity_checkables
> +    --->    ambiguity_checkables(
> +                % NOTTE We should consider making the types of the first

s/NOTTE/NOTE/

> +                % three fields type_ctor_defn_map, inst_ctor_defn_map and
> +                % mode_ctor_defn_map respectively. However, before we do that,
> +                % we need to decide exactly how we want to handle any entries
> +                % in the implementation section versions of those maps.
> +                % I (zs) think it is quite likely that the original code
> +                % of this module did not consider the treatment of such entries
> +                % thoroughly enough.
> +                %
> +                % Consider that the original motivation to put type
> +                % definitions into the implementation sections of .int files
> +                % was to give the compiler the information it needs to decide
> +                % on the correct representation of values of the type,
> +                % especially in the context of equivalence types involving
> +                % floats, which at the time were stored in two words
> +                % (as 64 bit entities on a 32 bit platform). However,
> +                % such type definition items specify non-user-visible
> +                % information, and as such should not be able to affect
> +                % which type names are ambiguous and which are not.
> +                % And yet the code of this module has always processed
> +                % type definition items without regard to which section
> +                % of an interface file they occurred in.

It's likely that the code in this module predates putting type definitions
into the implementation sections of .int files.

The diff looks fine other than the above.

Julien.