[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: position.hh compile error C4146 (VisualStudio 2017)
From: |
Akim Demaille |
Subject: |
Re: position.hh compile error C4146 (VisualStudio 2017) |
Date: |
Sat, 18 Aug 2018 17:03:21 +0200 |
Hi Rici!
> Le 18 août 2018 à 16:45, Rici Lake <address@hidden> a écrit :
>
> Hi, Akim
>
> I'd try
>
> return 0 < rhs || 0 - (static_cast<unsigned int>(rhs)) < lhs
Good idea!
> 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.
I don’t remember why I wrote it this way, but today I don’t care much about
INT_MIN here. I think the code has become too complex for what it meant to do.
Also, maybe I should have sticked to int instead of trying to unsigned, but
it’s too late to change that :)
> 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. :-)
Yes, absolutely. But I guess it’s too late too.
What do you think of my proposal restoring std::max?