Skip to content

Instantly share code, notes, and snippets.

@hershal
Last active September 17, 2016 17:52
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save hershal/c6cec363bb00ee363f8b1508d7fa4282 to your computer and use it in GitHub Desktop.
Save hershal/c6cec363bb00ee363f8b1508d7fa4282 to your computer and use it in GitHub Desktop.
/* This demonstrates that gcc or clang can incur a loss of precision of
variables without warning. Compile and run this program to see for yourself.
You may need to use `-std=c++11` depending on your compiler. Notice that the
size of some of the variables change. If you compile this with `-Wconversion`
then the compiler will properly complain about this conversion.
`-Wconversion` does not fall under `-Wall` or `-Wextra`. You must also use
`-Wconversion` to get the warning message. */
#include <iostream>
#include <stdint.h>
#include <math.h>
/* implicitly coerces the value to uint32_t during the return */
uint32_t implicit_conversion_return(uint64_t val) {
return val;
}
/* implicity coerces the argument to uin32_t during the function call */
uint64_t implicit_conversion_argument(uint32_t val) {
return val;
}
/* shouldn't do do any conversion */
uint64_t no_conversion(uint64_t val) {
return val;
}
int main() {
const uint64_t value = pow(2, 48) + 1;
const auto return_value = implicit_conversion_return(value);
const auto argument_value = implicit_conversion_argument(value);
const auto no_conversion_value = no_conversion(value);
std::cout << "original: " << sizeof(value) << ": " << value << std::endl
<< "return: " << sizeof(return_value) << ": " << return_value << std::endl
<< "argument: " << sizeof(argument_value) << ": " << argument_value << std::endl
<< "no conversion: " << sizeof(no_conversion_value) << ": " << no_conversion_value << std::endl;
return 0;
}
/*
# clang
I'm using clang bundled with LLVM 3.8.0 on Fedora 24
```
$ clang++ --version
clang version 3.8.0 (tags/RELEASE_380/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
```
## Without Wconversion
```
$ clang++ -std=c++11 -Wall -Wextra -Werror -fdiagnostics-color=always wconversion.cpp -o wconversion && ./wconversion
original: 8: 281474976710657
return: 4: 1
argument: 8: 1
no conversion: 8: 281474976710657
```
Note that clang doesn't error here, even though there is an obvious loss of
precision between the `original`, `return`, and `argument` values. The `return`
value is actually a standard 32-bit int, while `argument` gets truncated
## With Wconversion
```
$ clang++ -std=c++11 -Wall -Wextra -Werror -Wconversion -fdiagnostics-color=always wconversion.cpp -o wconversion && ./wconversion
wconversion.cpp:15:12: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
return val;
~~~~~~ ^~~
wconversion.cpp:29:39: error: implicit conversion turns floating-point number into integer: 'double' to 'const uint64_t' (aka 'const unsigned long') [-Werror,-Wfloat-conversion]
const uint64_t value = pow(2, 48) + 1;
~~~~~ ~~~~~~~~~~~^~~
wconversion.cpp:31:62: error: implicit conversion loses integer precision: 'const uint64_t' (aka 'const unsigned long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
const auto argument_value = implicit_conversion_argument(value);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~
3 errors generated.
```
We find the proper warnings.
# gcc
I'm using g++ 6.1.1 on Fedora 24.
```
$ g++ --version
g++ (GCC) 6.1.1 20160621 (Red Hat 6.1.1-3)
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
```
## Without Wconversion
```
g++ -std=c++11 -Wall -Wextra -Werror -fdiagnostics-color=always wconversion.cpp -o wconversion && ./wconversion
wconversion.cpp: In function ‘int main()’:
wconversion.cpp:31:67: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
const auto argument_value = implicit_conversion_argument(value);
^
cc1plus: all warnings being treated as errors
```
g++ seems to properly warn about the implicit conversion here, but only with the
argument. g++ still doesn't catch the `return` case. Funny enough, the warning
is emitted without Wall or Wextra, so that warning is compiled into this version
of g++.
## With Wconversion
```
$ g++ -std=c++11 -Wall -Wextra -Werror -Wconversion -fdiagnostics-color=always wconversion.cpp -o wconversion && ./wconversion
wconversion.cpp: In function ‘uint32_t implicit_conversion_return(uint64_t)’:
wconversion.cpp:15:12: error: conversion to ‘uint32_t {aka unsigned int}’ from ‘uint64_t {aka long unsigned int}’ may alter its value [-Werror=conversion]
return val;
^~~
wconversion.cpp: In function ‘int main()’:
wconversion.cpp:29:39: error: conversion to ‘uint64_t {aka long unsigned int}’ from ‘__gnu_cxx::__promote_2<int, int, double, double>::__type {aka double}’ may alter its value [-Werror=float-conversion]
const uint64_t value = pow(2, 48) + 1;
~~~~~~~~~~~^~~
wconversion.cpp:31:67: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
const auto argument_value = implicit_conversion_argument(value);
^
cc1plus: all warnings being treated as errors
```
We find the proper warnings.
# What have we learned?
In addition to `-Wall -Wextra -Werror`, one should also use `-Wconversion` to
ensure that nobody loses bits, time, and sanity debugging strange issues like
this. Personally I have spent many hours debugging a custom scheduler for my
homebrew real-time OS only to find out that I wrote too many zeroes in my init
routines for the ARM stack frame. I wish I had known about this flag in the
first place; I would have used it and caught the error immediately. This problem
could easily come up in production, and the expense would be much more costly
than a handful of debugging hours and sanity for that weekend.
Please advise: always use -Wconversion.
*/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment