[m-rev.] for review: do not hardcode the list of builtin primtive types in so many places

[Zoltan Somogyi]

> On 10 Apr 2017, at 00:50, Julien Fischer <jfischer at opturion.com> wrote:
> 
>    Add a comment document other places in the compiler that will need
>    to be updated if a new primitive type is added.

A word is missing here.

> compiler/add_foreign_enum.m:
> compiler/module_qual.qualify_items.m:
> compiler/special_pred.m:
> compiler/write_module_interface_files.m:
>     Use the new predicates rather than listing the primitive types
>     directly.

The key to the correctness of this change is not the modules that
have been updated to use the new predicates, but the modules that haven’t,
even though they should be. May I presume that you have looked at all
the modules in the compiler that mention all of “character”, “string”,
“int” and “float”?

> --- a/compiler/add_foreign_enum.m
> +++ b/compiler/add_foreign_enum.m
> @@ -55,12 +55,7 @@ add_pragma_foreign_export_enum(FEEInfo, _TypeStatus, Context,
>         % Emit an error message for foreign_export_enum pragmas for the
>         % builtin atomic types.
>         TypeArity = 0,
> -        ( TypeName = unqualified("character")
> -        ; TypeName = unqualified("float")
> -        ; TypeName = unqualified("uint")
> -        ; TypeName = unqualified("int")
> -        ; TypeName = unqualified("string")
> -        )
> +        is_builtin_type_sym_name(TypeName)

Indentation seems to be off, both here and below.

> index 14b0c9a..eeb14d6 100644
> --- a/compiler/prog_data.m
> +++ b/compiler/prog_data.m
> @@ -482,6 +482,16 @@ cons_id_is_const_struct(ConsId, ConstNum) :-
>             % A type expression with an explicit kind annotation.
>             % (These are not yet used.)
> 
> +
> +    % This type enumerates all of the builtin primitive types in Mercury.
> +    % If you add a new alternative then you may also need to update the
> +    % following predicates:
> +    %
> +    %     - parse_type_name.is_known_type_name_args/3
> +    %     - inst_check.check_inst_defn_has_matching_type/7
> +    %     - llds_out_data.output_type_ctor_addr/5
> +    %     - type_util.classify_type_ctor/2
> +    %
> :- type builtin_type
>     --->    builtin_type_int
>     ;       builtin_type_uint
> @@ -489,6 +499,10 @@ cons_id_is_const_struct(ConsId, ConstNum) :-
>     ;       builtin_type_string
>     ;       builtin_type_char.
> 
> +:- pred is_builtin_type_sym_name(sym_name::in) is semidet.
> +
> +:- pred is_builtin_type_name(string::in) is semidet.
> +
> 

I am surprised that there is no place in the compiler that would want
the builtin_ctor that corresponds to the name.

Coming back to this: I see that type_ctor_info.m has a version of this
code that does return the builtin_ctor. Is there any reason why you
haven’t moved that functionality here? It would reduce code duplication.

> --- a/compiler/special_pred.m
> +++ b/compiler/special_pred.m
> @@ -340,11 +340,7 @@ can_generate_special_pred_clauses_for_type(ModuleInfo, TypeCtor, TypeBody) :-
> is_builtin_type_special_preds_defined_in_mercury(TypeCtor, TypeName) :-
>     Builtin = mercury_public_builtin_module,
>     TypeCtor = type_ctor(qualified(Builtin, TypeName), 0),
> -    ( TypeName = "int"
> -    ; TypeName = "uint"
> -    ; TypeName = "string"
> -    ; TypeName = "character"
> -    ; TypeName = "float"
> +    ( is_builtin_type_name(TypeName)
>     ; TypeName = "pred"
>     ).

This code expects the symname to have builtin.m as the qualifier module.
The other code above all expected an *unqualified* name.

Either this code is wrong (and has been for a while), or the tests on
symnames in prog_data.m should accept both forms.

Otherwise, the diff is fine.

Zoltan.