On Thu, Apr 23, 2015 at 09:11:39PM -0600, Martin Sebor wrote:
> I wonder if the tests where the left shift operands are both
> constants really do invoke undefined behavior in GCC. For
> example, AFAICS, in (-1 << 0) and other constant expressions
> gcc computes the shift in unsigned HOST_WIDE_INT which is well
> defined.
 
Yeah, two INTEGER_CSTs are computed in wide-int using uhwi.  But -1 << 0
certainly is UB according to ISO C/C++ and I think we should follow the
standards.

> It seems the warning would be more valuable (and less likely
> dismissed as overly pedantic) if it was issued when the second
> operand was not constant and the computation had to be done in
> hardware (or even better, in hardware not known to  use the
> same instructions for positive and negative operands).
 
What I've tried to do in the patch was to mimic the other two Wshift-*
warnings.  While the new warning triggered a few times in the testsuite, GCC
bootstrap itself was clean.  You raise a very good point though, we don't
want to be overly pedantic.

I suppose we could go with the patch as-is, and if users complain too much,
warn only if the second operand is not constant or so.  But I hope that
left-shifting a negative value is not a common thing.

> The warning would also be valuable in some sort of a portability
> mode (with -pedantic?) where the code is intended to be portable
> to implementations that don't provide well-defined semantics for
> left shifts of negative values.

I really think -Wshift-negative-value should be kin to -Wshift-count-negative
and -Wshift-count-overflow, those are enabled by default.

Thanks,

        Marek

Reply via email to