[m-rev.] for review: Avoid 0xffffffff in code to shorten overlong identifiers.
Peter Wang
novalazy at gmail.com
Tue Apr 14 22:32:59 AEST 2020
On Tue, 14 Apr 2020 19:34:44 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
>
> 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.
Thanks. I will commit it with your suggestions shortly.
Peter
More information about the reviews
mailing list