llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.92k stars 11.52k forks source link

Missing warning for -Wconversion with compound assignment #11646

Open llvmbot opened 12 years ago

llvmbot commented 12 years ago
Bugzilla Link 11274
Version trunk
OS Linux
Reporter LLVM Bugzilla Contributor
CC @AaronBallman

Extended Description

A warning is not generated for the compound assignment case, though it is for standard operation + assignment.

Simple test case:

int main() { uint8_t a = 1; a <<= 1; // missing warning

uint8_t b = 1;
b = b << 1;  // warns correctly

}

Generates a warning only for b, but not for a:

conv.c:9:6: warning: implicit conversion loses integer precision: 'int' to 'uint8_t' (aka 'unsigned char') [-Wconversion] b = b << 1; ~ ^~

gcc v4.5 gets this right:

conv.c:6:2: warning: conversion to ‘uint8_t’ from ‘int’ may alter its value conv.c:9:2: warning: conversion to ‘uint8_t’ from ‘int’ may alter its value

By my reading of 6.5.16.2 Compound assignment, the two should be consistent. For the other operators, each operand shall have arithmetic type consistent with those allowed by the corresponding binary operator.

--

First bug report of any kind to llvm, I believe I've followed preferred style / info, please let me know if otherwise.

llvmbot commented 5 years ago

I ran into this issue so it still persists. Gcc and Visual Studio both generate warnings on both cases where clang only does one. This seems like a bug to me. Fwiw I ran into this on a program than can be reduced to:

include

int main() { uint32_t x = 2; int64_t y = -1; x = x + y; // warning in all x += y; // no warning only in clang }

Is there a plan to address this issue?

AaronBallman commented 12 years ago

There are a few ways to look at this.

1) How is a = a << 1 (and a <<= 1) any different from a *= 2, which does not warn?

2) The warning seems spurious on its face because the assignment goes right back to the original datatype

3) Then again, showing the warning does bring us more in line with gcc's behavior.

The standard doesn't actually help define the behavior here because it doesn't mandate any particular warning behavior in this case. However, self-consistency certainly makes sense.

So the question really becomes: should we remove the warning for a = a << 1? Then it would be consistent with <<=, and all of the other binary arithmetic operators.

AaronBallman commented 12 years ago

Please ignore my previous comment where I managed to close the wrong report! I've reopened this one.

AaronBallman commented 12 years ago

Works in ToT (r150306)

c:\llvm>clang -fsyntax-only -std=c++11 "C:\Users\Aaron Ballman\Desktop\test.cpp" C:\Users\Aaron Ballman\Desktop\test.cpp:10:3: error: no matching function for call to 'g' g(X<42>()); ^ C:\Users\Aaron Ballman\Desktop\test.cpp:6:13: note: candidate function [with T = 42] not viable: no known conversion from 'X<42>' to 'X<42> &' for 1st argument; inline void g(X& x) { ^ 1 error generated.

I get the same error in gcc.