8

I want to write a template that returns me the smallest signed integer type that can represent a given number. This is my solution:

/**
 * Helper for IntTypeThatFits.
 * Template parameters indicate whether the given number fits into 8, 16 or 32
 * bits. If neither of them is true, it is assumed that it fits 64 bits.
 */
template <bool fits8, bool fits16, bool fits32>
struct IntTypeThatFitsHelper { };

// specializations for picking the right type
// these are all valid combinations of the flags
template<> struct IntTypeThatFitsHelper<true, true, true> { typedef int8_t Result; };
template<> struct IntTypeThatFitsHelper<false, true, true> { typedef int16_t Result; };
template<> struct IntTypeThatFitsHelper<false, false, true> { typedef int32_t Result; };
template<> struct IntTypeThatFitsHelper<false, false, false> { typedef int64_t Result; };

/// Finds the smallest integer type that can represent the given number.
template <int64_t n>
struct IntTypeThatFits
{
    typedef typename IntTypeThatFitsHelper<
        (n <= numeric_limits<int8_t>::max()) && (n >= numeric_limits<int8_t>::min()), 
        (n <= numeric_limits<int16_t>::max()) && (n >= numeric_limits<int16_t>::min()), 
        (n <= numeric_limits<int32_t>::max()) && (n >= numeric_limits<int32_t>::min())
    >::Result Result;
};

However, GCC does not accept this code. I get an error "comparison is always true due to limited range of data type [-Werror=type-limits]". Why does that happen? n is a signed 64bit integer, and all of the comparisons may be true or false for different values of n, or am I overlooking something?

I will be glad for any help.

Edit: I should mention that I am using C++11.

Benjamin Schug
  • 269
  • 3
  • 12

5 Answers5

4

It's an issue with gcc, warnings in templated code can be frustrating. You can either change the warning or use another approach.

As you may know, templated code is analyzed twice:

  • once when first encountered (parsing)
  • once when instantiated for a given type/value

The problem here is that at instantiation, the check is trivial (yes 65 fits into an int thank you), and the compiler fails to realize that this warning does not hold for all instantiations :( It is very frustrating indeed for those of us who strive to have a warning-free compiling experience with warnings on.

You have 3 possibilities:

  • deactivate this warning, or demote it to a non-error
  • use a pragma to selectively deactivate it for this code
  • rework the code in another format so that it does not trigger the warning any longer

Note that sometimes the 3rd possibility involves a massive change and much more complicated solution. I advise against complicated one's code just to get rid of clueless warnings.

EDIT:

One possible workaround:

template <int64_t n>
struct IntTypeThatFits {
    static int64_t const max8 = std::numeric_limits<int8_t>::max();
    static int64_t const min8 = std::numeric_limits<int8_t>::min();

    static int64_t const max16 = std::numeric_limits<int16_t>::max();
    static int64_t const min16 = std::numeric_limits<int16_t>::min();

    static int64_t const max32 = std::numeric_limits<int32_t>::max();
    static int64_t const min32 = std::numeric_limits<int32_t>::min();

    typedef typename IntTypeThatFitsHelper<
        (n <= max8 ) && (n >= min8 ), 
        (n <= max16) && (n >= min16), 
        (n <= max32) && (n >= min32)
    >::Result Result;
};

... by changing the type of the data used in the comparison, it should silence the compiler warning. I suppose explicit casting (int64_t(std::numeric_limits<int8_t>::max())) could work too, but I found this more readable.

Matthieu M.
  • 251,718
  • 39
  • 369
  • 642
  • So this is a bug in GCC? After all, the warning only makes sense for runtime comparisons. – Benjamin Schug May 03 '12 at 15:58
  • @BenjaminSchug: I would not qualify it as a *bug* because you could argue it works as expected. After all in some cases you could **want** to be warned at instantiation time :x It is just that in your case it's harmful... – Matthieu M. May 03 '12 at 16:02
  • In this context, the comparison is _required_ to be a compile-time constant, otherwise I could not use it as a template argument. So if this is really the reason for the warning, how is that not a bug? – Benjamin Schug May 03 '12 at 16:07
  • @BenjaminSchug: I posted a work-around to the issue you experimented. The idea is to use the value provided by `numeric_limit::min` et al. but in a type that is more palatable for gcc's analyses. – Matthieu M. May 03 '12 at 16:18
  • @BenjaminSchug: In this precise context I agree it could probably be removed, but it's a bug only if the specs say it should behave otherwise ;) That being said, since gcc is open to discussion, you could perhaps signal it on their mailing list, they might be interested in this issue. – Matthieu M. May 03 '12 at 16:19
  • @BenjaminSchug I would consider this a bug in gcc, as the warning is clearly bogus. You get similar bogus warnings for shifts > size of operand in templated code, and they are considered bugs too. You should open a bug report for it. – Gunther Piez May 03 '12 at 16:22
  • This is a bug. See my answer: http://stackoverflow.com/questions/10434640/gcc-comparison-is-always-true-due-to-limited-range-of-data-type-in-template/10435458#10435458 – David Stone May 03 '12 at 16:27
  • @MatthieuM.: Your workaround works. :-) For the current situation, I will still stick to Boost, but in the general case, this is useful to know. – Benjamin Schug May 03 '12 at 17:09
  • +1 for "I advise against complicated changes just to get rid of clueless warnings" As the OP is using C++11 anyway, you could use `constexpr` instead of `const` – Gunther Piez May 03 '12 at 18:58
3

The error is happening because you asked GCC to give you errors about this warning with -Werror=type-limits. The warning -Wtype-limits gives you a warning if you ever do a comparison which will always be true due to the ranges of the given data types, for example:

uint8_t x;
if(x >= 0) { ... }  // always true, unsigned integers are non-negative
if(x >= 256) { ... }  // always false

int32_t x;
if(x < 9223372036854775808LL) { ... }  // always true

This warning can sometimes be useful, but in many cases including this it's just useless pedantry and can be ignored. It's normally a warning (enabled as part of -Wextra, if you use that), but with -Werror or -Werror=type-limits, GCC makes it an error.

Since in this case it's not actually indicative of a potential problem with your code, just turn off the warning with -Wno-type-limits, or make it not be an error with Werror=no-type-limits if you don't mind seeing those warnings in the compiler output.

Adam Rosenfield
  • 360,316
  • 93
  • 484
  • 571
  • Where in the OP's code is there a comparison that is always true due to the range of data types? – interjay May 03 '12 at 15:53
  • I don't want to disable that warning completely because it is useful in the normal case. Here it should just not trigger because that comparison is _supposed_ and _required_ to be a compile time constant. – Benjamin Schug May 03 '12 at 16:01
1
   typedef typename IntTypeThatFitsHelper<
        (n <= numeric_limits<int8_t>::max()) && (n >= numeric_limits<int8_t>::min()), 
        (n <= numeric_limits<int16_t>::max()) && (n >= numeric_limits<int16_t>::min()), 
        (n <= numeric_limits<int32_t>::max()) && (n >= numeric_limits<int32_t>::min())
    >::Result Result;

You can't do that in C++ (in C++11 you can) - numeric_limits<int8_t>::max() is not compile time constant. Are you using C++11?

BTW, Boost provides this for you already: http://www.boost.org/doc/libs/1_49_0/libs/integer/doc/html/boost_integer/integer.html

Anycorn
  • 46,748
  • 41
  • 153
  • 250
1

I think the other answers of what the problem is are wrong. I don't believe this is a case of an over-eager compiler, but I believe it's a compiler bug. This code still fires the warning:

template<int64_t n>
bool a() {
    return (n <= static_cast<int64_t>(std::numeric_limits<int8_t>::max()));
}

When calling a<500>();, but this code does not:

template<int64_t n>
bool a() {
    return (n <= static_cast<int64_t>(127));
}

std::numeric_limits::max() evaluates to 127. I'll file a bugzilla report for this later today if no one else does.

David Stone
  • 22,053
  • 14
  • 61
  • 77
0

You get the warning because for some instantations of template <int64_t n> struct IntTypeThatFits with small n (smaller than 2^32) some of the comparisons are always true (sic!) because of the range of the operand during compile time .

This could be considered a bogus warning in this case, because your code relies on it, OTOH you explictly requested to make it an error with a -Werror or similar command line switch, you basically get what you asked here.

Gunther Piez
  • 28,058
  • 6
  • 62
  • 101
  • *you basically get what you asked here* -> I disagree. It is a useful warning in many situations, and there are plenty of instantiations of the template where it will not fire. I wish compilers would avoid such warnings when they do not hold for every instantiations... though I do recognize it's complicated to evaluate. – Matthieu M. May 03 '12 at 15:59
  • IMHO generally using `-Werror` is just asking for trouble. Warnings are a good thing, but there are to many bogus warnings in gcc out there to make them erroes all the time. Especially with templated code evaluated during compile time. – Gunther Piez May 03 '12 at 16:21
  • I have no problems with `-Werror` in my code, and this is what my warning list looks like: http://stackoverflow.com/questions/5088460/flags-to-enable-thorough-and-verbose-g-warnings/9862800#9862800 (I occasionally turn on those other warnings I mention but say I don't use). – David Stone May 03 '12 at 16:29
  • @drhirsch: [Broken Window Theory](http://en.wikipedia.org/wiki/Broken_windows_theory) => as soon as you let warnings escape in the build, they propagate, until the few useful warnings (`-Wreturn` !!) are lost in the noise. I prefer to have a narrow, carefully selected list of warnings, and have them into errors, rather than heaps of useless warnings sprouted to the screen or no warning at all. – Matthieu M. May 03 '12 at 16:33
  • 1
    @DavidStone This is probably tightly related to coding style. For me, `-Wunused` is always on, defining a variable for later use is and not using it is a big no :-) OTOH `-pedantic` just wont work for me. – Gunther Piez May 03 '12 at 16:33
  • @MatthieuM. I fix _all_ my warnings, but I prefer to not let the compiler error out on a warning but instead let it finish the build. – Gunther Piez May 03 '12 at 16:35
  • I absolutely need -Werror because otherwise, the warnings get lost in a sea of `g++ -o build/debug/main.o -c -std=c++0x -Wall -Wextra -Wtons-of-other-warning-flags -g -pedantic src/main.cpp` `g++ -o build/optimized/main.o -c -std=c++0x -Wall -Wextra -Wtons-of-other-warning-flags -Ofast -march=native -funsafe-loop-optimizations -flto=4 -Wdisabled-optimization -DNDEBUG src/main.cpp` etc. – David Stone May 03 '12 at 19:10
  • @drhirsch: I've got too much output with my build, if I let it finish I don't have enough history available in my shell to spot the warnings ;) – Matthieu M. May 03 '12 at 19:34
  • Hmm, my IDE (eclipse) filters that for me and marks the lines with warnings and errors – Gunther Piez May 03 '12 at 19:35
  • @drhirsch: In my group, we have agreed that you should always fix any warnings before committing. The -Werror flag is a handy way of enforcing that agreement. Especially because this means that builds with warnings will show up as failures in the build server. ;-) – Benjamin Schug May 03 '12 at 19:55