[m-rev.] for review: correct types in java foreign procs

[Julien Fischer]

On Mon, 31 Aug 2009, Peter Wang wrote:

> Branches: main
>
> Expose correct types to Java foreign code, e.g. a variable with a non-foreign
> type like `list(T)' is now visible to Java foreign code with the type
> `list.List_1' instead of `java.lang.Object'.
>
> A complication is that foreign code for mutables is generated in the compiler
> frontend, which doesn't know about the types used by the backends.  For Java,
> mutable variables had the generic type `Object' (except for primitives types)
> but as we now generate foreign procs with the correct variable types, the code
> for accessing a mutable would be type incorrect, e.g.
>
>        X = mutable_var;  /* list.List_1 = Object */
>
> Here we handle generated mutable predicates as a special case, using `Object'
> for the global variable and argument types and generating typecasts as before.
> The correct fix would be to move mutable code generation into the backends.

Yes, there are a bunch of bugs, #19 for example, that also require that fix.
(If you are contemplating making such a fix, we should discuss it beforehand.)

> compiler/make_hlds_passes.m:
>        Make Java mutable variables always have the type `java.lang.Object'.
>
>        Mark Java mutable predicates with `may_not_duplicate' attributes, as
>        the generated code doesn't contain class-qualifiers for the mutable
>        variable, so copying that code into other Java files won't work.
> compiler/mlds.m:
>        Add a new target code component `target_code_type(Type)' which is a
>        placeholder for a type, to be substituted by the backend when target
>        code is being printed out.
>
> compiler/ml_code_gen.m:
>        Generate Java foreign procs using `target_code_type' to declare local
>        variables, except for mutable predicates as described above.
>
>        Rename a predicate shared for C and Java.
>
>        Assign dummy type variables the value `null' in the Java backend
>        instead of `0'.
>
> compiler/foreign.m:
>        Add a comment for `exported_type_to_string'.
>
> compiler/mlds_to_java.m:
>        Handle the addition of `target_code_type'.
>
>        Output `java.lang.Output' for builtin dummy types.  We were
>        inconsistent as to whether builtin dummy types should be represented by
>        `int' or `Object'.
>
>        Output line number context in another spot.
>
> compiler/ml_elim_nested.m:
> compiler/ml_optimize.m:
> compiler/ml_util.m:
> compiler/mlds_to_c.m:
> compiler/mlds_to_il.m:
>        Conform to the addition of `target_code_type'.
>
> library/exception.m:
> library/io.m:
> library/list.m:
> library/rtti_implementation.m:
> library/string.m:
> library/type_desc.m:
>        Delete casts in some Java foreign code which are no longer required.
>
> diff --git a/compiler/foreign.m b/compiler/foreign.m
> index d289bab..947e4ff 100644
> --- a/compiler/foreign.m
> +++ b/compiler/foreign.m
> @@ -745,6 +745,8 @@ exported_type_to_string(Lang, ExportedType) = Result :-
>                 ; Type = type_variable(_, _)
>                 ; Type = kinded_type(_, _)
>                 ),
> +                % This is here so we can share some code between C and Java
> +                % backends.  This is not the correct type to use in general.
>                 Result = "java.lang.Object"
>             )
>         ;
> diff --git a/compiler/make_hlds_passes.m b/compiler/make_hlds_passes.m
> index 849055f..1a84c3d 100644
> --- a/compiler/make_hlds_passes.m
> +++ b/compiler/make_hlds_passes.m
> @@ -2311,12 +2311,8 @@ add_java_mutable_defn(TargetMutableName, Type, IsConstant,
> :- pred get_java_mutable_global_foreign_defn(module_info::in, mer_type::in,
>     string::in, bool::in, prog_context::in, item::out) is det.
>
> -get_java_mutable_global_foreign_defn(ModuleInfo, Type, TargetMutableName,
> +get_java_mutable_global_foreign_defn(_ModuleInfo, _Type, TargetMutableName,
>         IsConstant, Context, DefnItem) :-
> -    module_info_get_globals(ModuleInfo, Globals),
> -    globals.lookup_bool_option(Globals, mutable_always_boxed, AlwaysBoxed),
> -    TypeName = global_foreign_type_name(AlwaysBoxed, lang_java, ModuleInfo,
> -        Type),
>     MutableMutexVarName = mutable_mutex_var_name(TargetMutableName),
>
>     % Constant mutables do not require mutexes as their values are never
> @@ -2331,7 +2327,7 @@ get_java_mutable_global_foreign_defn(ModuleInfo, Type, TargetMutableName,
>     ),
>
>     DefnBody = string.append_list([
> -        "static ", TypeName, " ", TargetMutableName, ";\n" | LockDefn]),
> +        "static java.lang.Object ", TargetMutableName, ";\n" | LockDefn]),
>     DefnPragma = pragma_foreign_code(lang_java, DefnBody),
>     DefnItemPragma = item_pragma_info(compiler(mutable_decl), DefnPragma,
>         Context, -1),
> @@ -2348,7 +2344,11 @@ add_java_mutable_preds(ItemMutable, TargetMutableName,
>     ItemMutable = item_mutable_info(MercuryMutableName, _Type, InitTerm, Inst,
>         MutAttrs, MutVarset, Context, _SeqNum),
>     IsConstant = mutable_var_constant(MutAttrs),
> -    Attrs = default_attributes(lang_java),
> +    Attrs0 = default_attributes(lang_java),
> +    % The mutable variable name is not module-qualified so cannot be exported
> +    % to `.opt' files. We could add the qualification but it would be better
> +    % to move the mutable code generation into the backends first.
> +    set_may_duplicate(yes(proc_may_not_duplicate), Attrs0, Attrs),
>     module_info_get_globals(!.ModuleInfo, Globals),
>     globals.lookup_bool_option(Globals, mutable_always_boxed, AlwaysBoxed),
>     (
> diff --git a/compiler/ml_code_gen.m b/compiler/ml_code_gen.m
> index 801349a..608ef55 100644
> --- a/compiler/ml_code_gen.m
> +++ b/compiler/ml_code_gen.m
> @@ -2382,10 +2382,10 @@ ml_gen_nondet_pragma_foreign_proc(CodeModel, Attributes, PredId, _ProcId,
>         ], HashUndefs),
>
>     % Generate code to set the values of the input variables.
> -    ml_gen_pragma_c_input_arg_list(Lang, Args, AssignInputsList, !Info),
> +    ml_gen_pragma_c_java_input_arg_list(Lang, Args, AssignInputsList, !Info),
>
>     % Generate code to assign the values of the output variables.
> -    ml_gen_pragma_c_output_arg_list(Lang, Args, Context,
> +    ml_gen_pragma_c_output_arg_list(Args, Context,
>         AssignOutputsList, ConvDecls, ConvStatements, !Info),
>
>     % Generate code fragments to obtain and release the global lock.
> @@ -2601,21 +2601,29 @@ ml_gen_ordinary_pragma_foreign_proc(CodeModel, Attributes, PredId, ProcId,
>     prog_context::in, list(mlds_defn)::out, list(statement)::out,
>     ml_gen_info::in, ml_gen_info::out) is det.
>
> -ml_gen_ordinary_pragma_java_proc(_CodeModel, Attributes, _PredId, _ProcId,
> +ml_gen_ordinary_pragma_java_proc(_CodeModel, Attributes, PredId, _ProcId,
>         Args, ExtraArgs, JavaCode, Context, Decls, Statements, !Info) :-
> -
>     Lang = get_foreign_language(Attributes),
>
> +    ml_gen_info_get_module_info(!.Info, ModuleInfo),
> +    module_info_pred_info(ModuleInfo, PredId, PredInfo),
> +    pred_info_get_markers(PredInfo, Markers),
> +    ( check_marker(Markers, marker_mutable_access_pred) ->
> +        MutableSpecial = mutable_special_case
> +    ;
> +        MutableSpecial = not_mutable_special_case
> +    ),
> +
>     % Generate <declaration of one local variable for each arg>
> -    ml_gen_pragma_c_decls(!.Info, Lang, Args, ArgDeclsList),
> +    ml_gen_pragma_java_decls(!.Info, MutableSpecial, Args, ArgDeclsList),
>     expect(unify(ExtraArgs, []), this_file,
>         "ml_gen_ordinary_pragma_java_proc: extra args"),
>
>     % Generate code to set the values of the input variables.
> -    ml_gen_pragma_c_input_arg_list(Lang, Args, AssignInputsList, !Info),
> +    ml_gen_pragma_c_java_input_arg_list(Lang, Args, AssignInputsList, !Info),
>
>     % Generate MLDS statements to assign the values of the output variables.
> -    ml_gen_pragma_java_output_arg_list(Lang, Args, Context,
> +    ml_gen_pragma_java_output_arg_list(MutableSpecial, Args, Context,
>         AssignOutputsList, ConvDecls, ConvStatements, !Info),
>
>     % Put it all together
> @@ -2971,10 +2979,10 @@ ml_gen_ordinary_pragma_c_proc(OrdinaryKind, Attributes, PredId, _ProcId,
>     ml_gen_pragma_c_decls(!.Info, Lang, Args, ArgDeclsList),
>
>     % Generate code to set the values of the input variables.
> -    ml_gen_pragma_c_input_arg_list(Lang, Args, AssignInputsList, !Info),
> +    ml_gen_pragma_c_java_input_arg_list(Lang, Args, AssignInputsList, !Info),
>
>     % Generate code to assign the values of the output variables.
> -    ml_gen_pragma_c_output_arg_list(Lang, Args, Context,
> +    ml_gen_pragma_c_output_arg_list(Args, Context,
>         AssignOutputsList, ConvDecls, ConvStatements, !Info),
>
>     % Generate code fragments to obtain and release the global lock.
> @@ -3142,9 +3150,6 @@ get_target_code_attributes(Lang, [ProcAttr | ProcAttrs]) = TargetAttrs :-
> :- pred ml_gen_pragma_c_decls(ml_gen_info::in, foreign_language::in,
>     list(foreign_arg)::in, list(target_code_component)::out) is det.
>
> -    % XXX Maybe this ought to be renamed as it works for, and
> -    % is used by the Java back-end as well.
> -    %
> ml_gen_pragma_c_decls(_, _, [], []).
> ml_gen_pragma_c_decls(Info, Lang, [Arg | Args], [Decl | Decls]) :-
>     ml_gen_pragma_c_decl(Info, Lang, Arg, Decl),
> @@ -3181,6 +3186,55 @@ ml_gen_pragma_c_decl(Info, Lang, Arg, Decl) :-
>
> %-----------------------------------------------------------------------------%
>
> +:- type mutable_special_case
> +    --->    mutable_special_case
> +    ;       not_mutable_special_case.

Document this type.

The change looks fine otherwise.

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