[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