[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