Re: position.hh compile error C4146 (VisualStudio 2017)

[Rici Lake]

Hi, Akim

I'd try

    return 0 < rhs || 0 - (static_cast<unsigned int>(rhs)) < lhs

Or the redundant but harmless

    return 0 < rhs || static_cast<unsigned int>(-(static_cast<unsigned
int>(rhs))) < lhs

C4146 is a warning, not an error. It's telling you that if u is unsigned,
-u is still unsigned. The intent is help people avoid doing things like:

    if ( i > -2147483648) ...

which is broken (with 32-bit integers, assuming i is an int) because the
constant is an unsigned int and therefore the comparison will be unsigned,
which will have unexpected results. Or, at least, MS assumes the results
will be unexpected.

As I understand it, your code is precisely trying to avoid the undefined
behaviour possible when using -rhs, if rhs happens to be equal to INT_MIN.
I don't see any problem with it, but you have to convince Visual Studio
that you know what you're doing.

By the way, contrary to the claim in the comment in add_, that function
only works if min is 0 or 1. If min were 2, the return value could be 1. If
the intent is that b4_location_initial_column and b4_location_initial_line
only have 0 or 1 as possible values, it might be clearer to make them
booleans with names like b4_location_column_is_one_based and
b4_location_line_is_one_based. :-)

Rici

2018-08-18 8:53 GMT-05:00 長田偉伸 <address@hidden>:

> > Sorry, it is not clear to me: it still works, or it is still broken?
>
> it is still broken.
>
> > Can I install this fix?
>
> > Would you also consider reporting this bug to MS?
>
> Sorry, I can not do anything.
>
> Because I can understand English only a little.
> (I am Japanese)
>
>
>
> 2018-08-18 22:40 GMT+09:00 Akim Demaille <address@hidden>:
> > Please, always keep the list in CC.
> >
> >> Le 18 août 2018 à 15:26, 長田偉伸 <address@hidden> a écrit :
> >>
> >> Thank you for your early reply
> >>
> >> I changed the file and then compiled it.
> >> However, the situation has not changed.
> >
> > Sorry, it is not clear to me: it still works, or it is still broken?
> >
> >> ----- position.hh: 107 - 114
> >>    /// Compute max(min, lhs+rhs) (provided min <= lhs).
> >>    static unsigned add_ (unsigned lhs, int rhs, unsigned min)
> >>    {
> >>      //return (0 < rhs || -static_cast<unsigned>(rhs) < lhs
> >>      return (0 < rhs || -(static_cast<unsigned int>(rhs)) < lhs
> >>              ? rhs + lhs
> >>              : min);
> >>    }
> >> ——
> >
> > Can I install this fix?
> >
> > Would you also consider reporting this bug to MS?  Thanks!
>
>