Project

General

Profile

Bug #13992

cetlib::ston causes tautological-compare error on Clang

Added by Ben Morgan over 3 years ago. Updated almost 3 years ago.

Status:
Closed
Priority:
Normal
Target version:
Start date:
10/03/2016
Due date:
% Done:

100%

Estimated time:
Duration:

Description

Building cetlib on macOS (El Cap, Xcode 7, 8), a tautological compare warning (promoted to error by CET default flags) is emitted by cetlib::ston specialised on unsigned

[ 65%] Building CXX object cetlib/test/CMakeFiles/ston_test.dir/ston_test.cc.o
In file included from /.../cetlib/test/ston_test.cc:4:
/.../cetlib/ston.h:90:17: error: comparison of unsigned expression < 0 is always false
      [-Wtautological-compare]
  if(    result < std::numeric_limits<unsigned>::min()
         ~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Clang clearly has higher default warnings than GCC, but I think this is valid as the code block in question is:

template<>
unsigned
  cet::ston<unsigned>( std::string s )
{
  std::size_t size;
  unsigned long result = stoul(s, &size);
  if( size < s.size() )
    throw std::invalid_argument("ston<unsigned long>");
  if(    result < std::numeric_limits<unsigned>::min()
      || std::numeric_limits<unsigned>::max() < result
    )
    throw std::out_of_range("ston<unsigned long>");
  return static_cast<unsigned>(result);
}

result is defined as unsigned long, so I think that guarantees it to have the same min value as unsigned, i.e. zero. Changing the code to

template<>
unsigned
  cet::ston<unsigned>( std::string s )
{
  std::size_t size;
  unsigned long result = stoul(s, &size);
  if( size < s.size() )
    throw std::invalid_argument("ston<unsigned long>");
  if( std::numeric_limits<unsigned>::max() < result )
    throw std::out_of_range("ston<unsigned long>");
  return static_cast<unsigned>(result);
}

has the same behaviour. At least, it compiles and passes the tests.

History

#1 Updated by Kyle Knoepfel over 3 years ago

  • Status changed from New to Accepted

As previously proposed to the art-users/stakeholders, we will remove the cet::ston and cet::ntos facilities as they are not used anywhere in user code, and equivalent facilities are provided by the C++11/14 standards.

#2 Updated by Christopher Green almost 3 years ago

  • Status changed from Accepted to Resolved
  • Assignee set to Kyle Knoepfel
  • % Done changed from 0 to 100

Resolved with 15b825b9e916b6e8b3edcdba8985a0f64cd7e3aa. Released in art 2.05.00.

#3 Updated by Kyle Knoepfel almost 3 years ago

  • Status changed from Resolved to Closed
  • Assignee changed from Kyle Knoepfel to Christopher Green
  • Target version set to 2.05.00


Also available in: Atom PDF