Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC 5.3.1 Compiler warning: sign compare #16

Closed
rph-pl opened this issue May 30, 2016 · 6 comments
Closed

GCC 5.3.1 Compiler warning: sign compare #16

rph-pl opened this issue May 30, 2016 · 6 comments

Comments

@rph-pl
Copy link

rph-pl commented May 30, 2016

Compiler:
g++ (Debian 5.3.1-21) 5.3.1 20160528

Code:

#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include "doctest/doctest.h"

#include <map>

TEST_CASE("x")
{
    std::map<int, int> m;
    CHECK(m.size() == 0);
}

Command line (unimportant flags like include paths removed):

/usr/bin/c++ -std=c++14 -Wall -Wextra -Wshadow -Wnon-virtual-dtor -pedantic -Wcast-align
-Woverloaded-virtual -g -fprofile-arcs -ftest-coverage -fno-omit-frame-pointer -O0 
-std=gnu++14
-o CMakeFiles/x.dir/tests/x.cpp.o

Result (full paths removed):

In file included from /somewhere/tests/x.cpp:2:0:
/somewhere/tests/doctest/doctest.h: In instantiation of ‘typename doctest::detail::enable_if<(doctest::detail::can_use_op<T>::value || doctest::detail::can_use_op<R>::value), bool>::type doctest::detail::eq(const L&, const R&) [with L = long unsigned int; R = int; typename doctest::detail::enable_if<(doctest::detail::can_use_op<T>::value || doctest::detail::can_use_op<R>::value), bool>::type = bool]’:
/somewhere/tests/doctest/doctest.h:593:82:   required from ‘doctest::detail::Result doctest::detail::Expression_lhs<L>::operator==(const R&) [with R = int; L = const long unsigned int&]’
/somewhere/tests/x.cpp:9:2:   required from here
/somewhere/tests/doctest/doctest.h:557:127: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  || can_use_op<R>::value, bool>::type eq (const L& lhs, const R& rhs) { return lhs == rhs; }
                                                                                    ^
@martinmoene
Copy link
Contributor

martinmoene commented May 30, 2016

Well the compiler is right isn't it? Using m.size() == 0u obviates the warning.

Edit:
size_type size() const;
size_type:Unsigned integral type (usually std::size_t)

@onqtam
Copy link
Member

onqtam commented May 30, 2016

well... I don't get a warning for a normal if statement like if(m.size() == 0) and also Catch doesn't produce a warning for CHECK(m.size() == 0) so I should look into this...

@martinmoene
Copy link
Contributor

With GNU 5.2.0 on Windows I also get the warning for m.size() == 0.

@martinmoene
Copy link
Contributor

martinmoene commented May 30, 2016

Catch actively suppresses signed/unsigned mismatch for the expression decomposer.

Edit: ... for MSVC

@martinmoene
Copy link
Contributor

martinmoene commented May 30, 2016

@onqtam onqtam mentioned this issue Jun 23, 2016
@onqtam
Copy link
Member

onqtam commented Sep 13, 2016

So I asked these 2 questions on SO and didn't get the nice explanation I was hoping for.

Atleast I have a conclusion: if(vec.size() == 0) does not produce a warning, but CHECK(vec.size() == 0) does - and the reason is that I bind the 2 values to const references in the expression decomposing template machinery.

Here is a summary that shows the issue:

  • gives a warning:
int a = 0;
unsigned b = 0;
if(a == b)
  • does NOT give a warning:
const int a = 0;
const unsigned b = 0;
if(a == b)
  • gives a warning:
const int& a = 0;
const unsigned& b = 0;
if(a == b)

I don't like the way Catch handles this in gcc/clang - all those templates and specializations must have a compile time performance overhead.

The way I've chosen is to disable the few relevant warnings in the templated code, and the user will be able to remove these warning suppressions with the DOCTEST_CONFIG_NO_COMPARISON_WARNING_SUPPRESSION identifier if they wish to be pedantic.

The downside of this will be that code that should generate a warning will not by default (unless that identifier is defined before the inclusion of doctest)

onqtam added a commit that referenced this issue Sep 13, 2016
…e machinery - comparing signed to unsigned integers should now not generate warnings...

If this behavior is not desired the user may use the DOCTEST_CONFIG_NO_COMPARISON_WARNING_SUPPRESSION identifier to remove the suppression of warnings.

This was done mainly so code like CHECK(vec.size() == 0) would compile cleanly.

fixed #16
@onqtam onqtam closed this as completed in f90739e Sep 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants