[m-rev.] for review: delete contains_foreign_* from module_and_imports

Julien Fischer jfischer at opturion.com
Fri Aug 6 13:00:19 AEST 2021


Hi Zoltan,

On Fri, 6 Aug 2021, Zoltan Somogyi wrote:

> Delete module_and_imports' contains_foreign_* fields.
> 
> compiler/module_imports.m:
>     As above. The new approach has a much simpler correctness argument,
>     especially since some of the methods for initializing module_and_imports
>     structures don't actually fill in one of the fields with meaningful
>     information.
>
>     Don't ask callers for arguments that are not used.
> 
> compiler/get_dependencies.m:
>     Add utility predicates for computing the information that those fields
>     used to contain on demand.
>
>     Simplify the interface to some similar recently-added utility predicates.
> 
> compiler/make.module_dep_file.m:
>     Call those utility predicates when needed.
> 
> compiler/item_util.m:
>     Delete the utility predicates that module_imports.m *used* to use
>     to fill in those fields, which is no longer needed.
> 
> compiler/grab_modules.m:
> compiler/write_deps_file.m:
>     Conform to the changes above.



> diff --git a/compiler/get_dependencies.m b/compiler/get_dependencies.m
> index 2def40d48..2933bdcfe 100644
> --- a/compiler/get_dependencies.m
> +++ b/compiler/get_dependencies.m

...

> @@ -1490,6 +1520,85 @@ acc_fact_tables_from_impl_pragma(ItemImplPragma, !FactTables) :-
>          )
>      ).

...

> +        ; ImplPragma = impl_pragma_require_feature_set(_)
> +        )
> +    ).
> +
> +:- pred acc_foreign_code_langs_from_impl_pragma(item_impl_pragma_info::in,
> +    set(foreign_language)::in, set(foreign_language)::out) is det.
> +
> +acc_foreign_code_langs_from_impl_pragma(ItemImplPragma, !Langs) :-
> +    ItemImplPragma = item_pragma_info(ImplPragma, _, _),
> +    (
> +        (
> +            ImplPragma = impl_pragma_foreign_code(FCInfo),
> +            FCInfo = pragma_info_foreign_code(Lang, _LiteralOrInclude)
> +        ;
> +            ImplPragma = impl_pragma_foreign_proc(FPInfo),
> +            FPInfo = pragma_info_foreign_proc(Attrs, _Name, _, _, _, _, _),
> +            Lang = get_foreign_language(Attrs)
> +            % NOTE We used to keep a record of the set of foreign languages
> +            % in which there was a foreign_proc for a given procedure,
> +            % and then chose the one in the *preferred* foreign language.
> +            % This made sense when we had the IL backend, which could handle
> +            % foreign procs in both IL and C#, but as of Aug 2021, when
> +            % we target a foreign language, we accept for it only the
> +            % foreign procs in that language, so this mechanism is unnecessary.

Certainly, it's unecessary at the moment; previously I had retained the residual
support for multiple foreign language in case we did want to revive it.
(That's not an objetion to this diff since the comment makes it clear what
needs to happen if such support is revived.)

> +        ;
> +            ImplPragma = impl_pragma_foreign_proc_export(FPEInfo),
> +            FPEInfo = pragma_info_foreign_proc_export(_, Lang, _, _)
> +        ;
> +            ImplPragma = impl_pragma_fact_table(_),
> +            Lang = lang_c
> +        ),
> +        set.insert(Lang, !Langs)
> +    ;
> +        % We do NOT count foreign_decls here. We only link in a foreign object
> +        % file if mlds_to_gcc called mlds_to_c.m to generate it, which it
> +        % will only do if there is some foreign_code, not just foreign_decls.

mlds_to_gcc was deleted along with the rest of the GCC backend, so that bit
is no longer relevant.

The diff is fine.

Julien.


More information about the reviews mailing list