[m-rev.] for review: mlds_to_{cs,java}_type.m

Julien Fischer jfischer at opturion.com
Sat May 6 02:02:29 AEST 2023


On Wed, 3 May 2023, Zoltan Somogyi wrote:

> Get mlds_to_{cs,java}_type.m closer to each other ...
> 
> ... and simplify them.

...

> diff --git a/compiler/mlds_to_cs_data.m b/compiler/mlds_to_cs_data.m
> index c9138da65..620f326bc 100644
> --- a/compiler/mlds_to_cs_data.m
> +++ b/compiler/mlds_to_cs_data.m
> @@ -317,14 +316,18 @@ output_cast_rval_for_csharp(Info, Type, Expr, Stream, !IO) :-
>          output_rval_for_csharp(Info, Expr, Stream, !IO),
>          io.write_string(Stream, ")", !IO)
>      else if
> +        % Given that the then-part and else-part of this if-then-else
> +        % do exactly the same thing when the test succeeds, its only purpose
> +        % can be optimization. However, I (zs) have strong doubts about
> +        % whether this attempt at optimization actually works, because
> +        % the gain, even when we get it, is very small.

It's not an optimization. The code was inherited from the Java backend (see
below) where we need to add a field access if the r-value is actually an enum.
This is because the Java backend currently represents enumerations as objects,
whereas the C# represents them using C# enumerations and these can be converted
to ints via a cast.

>          csharp_builtin_type(Type, "int")
>      then
>          io.write_string(Stream, "(int) ", !IO),
>          output_rval_for_csharp(Info, Expr, Stream, !IO)
>      else
> -        io.write_string(Stream, "(", !IO),
> -        output_type_for_csharp(Info, Type, Stream, !IO),
> -        io.write_string(Stream, ") ", !IO),
> +        io.format(Stream, "(%s) ",
> +            [s(type_to_string_for_csharp(Info, Type))], !IO),
>          output_rval_for_csharp(Info, Expr, Stream, !IO)
>      ).

...

> diff --git a/compiler/mlds_to_java_data.m b/compiler/mlds_to_java_data.m
> index 935c364b6..00072817d 100644
> --- a/compiler/mlds_to_java_data.m
> +++ b/compiler/mlds_to_java_data.m

...

> @@ -330,21 +329,25 @@ output_cast_rval_for_java(Info, Type, Expr, Stream, !IO) :-
>          % XXX We really should be able to tell if we are casting a
>          % TypeCtorInfo or a TypeInfo. Julien says that's probably going to
>          % be rather difficult as the compiler doesn't keep track of where
> -        % type_ctor_infos are acting as type_infos properly.
> +        % type_ctor_infos are acting as type_infos properly. (zs agrees.)
>          maybe_output_comment_for_java(Info, Stream, "cast", !IO),
>          io.write_string(Stream,
>              "jmercury.runtime.TypeInfo_Struct.maybe_new(", !IO),
>          output_rval_for_java(Info, Expr, Stream, !IO),
>          io.write_string(Stream, ")", !IO)
>      else if
> +        % Given that the then-part and else-part of this if-then-else
> +        % do exactly the same thing when the test succeeds, its only purpose
> +        % can be optimization. However, I (zs) have strong doubts about
> +        % whether this attempt at optimization actually works, because
> +        % the gain, even when we get it, is very small.

That comment is not correct; special handling is required when the r-value
is an enum.

>          java_builtin_type(Type, "int", _, _)
>      then
>          io.write_string(Stream, "(int) ", !IO),
>          output_rval_maybe_with_enum_for_java(Info, Expr, Stream, !IO)
>      else
> -        io.write_string(Stream, "(", !IO),
> -        output_type_for_java(Info, Type, Stream, !IO),
> -        io.write_string(Stream, ") ", !IO),
> +        io.format(Stream, "(%s) ",
> +            [s(type_to_string_for_java(Info, Type))], !IO),
>          output_rval_for_java(Info, Expr, Stream, !IO)
>      ).

Julien.


More information about the reviews mailing list