[m-rev.] for review: Avoid 0xffffffff in code to shorten overlong identifiers.

Julien Fischer jfischer at opturion.com
Tue Apr 14 19:34:44 AEST 2020


Hi Peter,

On Tue, 14 Apr 2020, Peter Wang wrote:

> compiler/c_util.m:
>    Add hex_hash32 function to replace similar code in the following
>    three modules. Avoid using 0xffffffff as that constant may cause the
>    C compiler to emit warnings.
>
> elds_to_erlang.m:
> mlds_to_cs_name.m:
> mlds_to_java_class.m:
>    Use hex_hash32.
>
> diff --git a/compiler/c_util.m b/compiler/c_util.m
> index d131a8526..6fafc4362 100644
> --- a/compiler/c_util.m
> +++ b/compiler/c_util.m
> @@ -2,7 +2,7 @@
> % vim: ft=mercury ts=4 sw=4 et
> %---------------------------------------------------------------------------%
> % Copyright (C) 1999-2007, 2009-2012 The University of Melbourne.
> -% Copyright (C) 2013-2018 The Mercury team.
> +% Copyright (C) 2013-2020 The Mercury team.
> % This file may only be copied under the terms of the GNU General
> % Public License - see the file COPYING in the Mercury distribution.
> %---------------------------------------------------------------------------%
> @@ -324,6 +324,17 @@
>     %
> :- pred is_valid_c_identifier(string::in) is semidet.
> 
> +%---------------------------------------------------------------------------%
> +%
> +% Utility predicate to shorten overlong identifiers.
> +%
> +
> +    % Return hexadecimal encoded hash of a string.
> +    % The resulting string has a length of 8 characters, and it must be
> +    % consistent across different compiler backends and bit widths.

I suggest:

      The resulting string has a length of 8 characters and will be
      consistent across different compiler backends and word sizes.

> +    %
> +:- func hex_hash32(string) = string.
> +
> %---------------------------------------------------------------------------%
> %---------------------------------------------------------------------------%
> 
> @@ -1119,6 +1130,16 @@ is_valid_c_identifier(S) :-
>     char.is_alpha_or_underscore(Start),
>     string.is_all_alnum_or_underscore(S).
> 
> +%---------------------------------------------------------------------------%
> +
> +hex_hash32(S0) = S :-
> +    Hash = string.hash(S0),
> +    % Mask off the lower 32 bits without using 0xffffffff in the generated
> +    % target code.

I would add the rationale for doing this to the comment, i.e. we do this to
avoid warnings from the C compiler.

> +    Hi = (Hash >> 16) /\ 0xffff,
> +    Lo = Hash /\ 0xffff,
> +    S = string.format("%04x%04x", [i(Hi), i(Lo)]).

Looks fine otherwise.

Julien.


More information about the reviews mailing list