Add explicit shared_ptr constructor due to C++11 error #6899

mcelrath commented at 11:49 PM on October 28, 2015: none

Clean up some warnings (and one error) when compiling in C++11 mode with g++ and clang. Details:

-Wno-self-assign is no longer a valid warning to disable
All the stuff in leveldb is cases of comparison between signed and unsigned
The change in rpcserver.cpp is an error with gcc 5.2, apparently older versions can do an automatic conversion here but in C++11 mode it needs an explicit conversion to shared_ptr.

There are other warnings due to a few uses of std::auto_ptr which is deprecated. When we switch to compiling in C++11 mode by default, we'll have to convert these to std::unique_ptr, but this is a non-backwards compatible change so can't be done now.

dcousens commented at 12:04 AM on October 29, 2015: contributor

utACK

in src/leveldb/include/leveldb/filter_policy.h:None in d18c720861 outdated

  37 | @@ -38,7 +38,7 @@ class FilterPolicy {
  38 |    //
  39 |    // Warning: do not change the initial contents of *dst.  Instead,
  40 |    // append the newly constructed filter to *dst.
  41 | -  virtual void CreateFilter(const Slice* keys, int n, std::string* dst)
  42 | +  virtual void CreateFilter(const Slice* keys, unsigned int n, std::string* dst)
  43 |        const = 0;

dcousens commented at 12:05 AM on October 29, 2015:

Heh, on its own, this just looks like const = 0.

in configure.ac:None in d18c720861 outdated

 177 | @@ -178,7 +178,7 @@ fi
 178 |  ##       compatibility with the legacy buildsystem.
 179 |  ##
 180 |  if test "x$CXXFLAGS_overridden" = "xno"; then
 181 | -  CXXFLAGS="$CXXFLAGS -Wall -Wextra -Wformat -Wformat-security -Wno-unused-parameter -Wno-self-assign"

dcousens commented at 12:06 AM on October 29, 2015:

Wasn't this added recently? Reference #6744

gmaxwell commented at 12:14 AM on October 29, 2015: contributor

I really do not want to carry around and maintain a patch for leveldb for superfluous warnings.

jgarzik commented at 12:40 AM on October 29, 2015: contributor

Agree w/ @gmaxwell - we want to avoid patching leveldb

dcousens commented at 1:00 AM on October 29, 2015: contributor

@mcelrath, given the above, maybe just the rpcserver change?

mcelrath commented at 2:10 AM on October 29, 2015: none

Dunno, I wouldn't want leveldb in the core in the first place...

I haven't analyzed these sign comparisons to see if they could overflow, but I find the idea pretty scary. But I don't think this is an argument worth having. Will backout everything except the rpcserver change. We can remove the -Wno-self-assign when we actually switch to C++11.

Add explicit shared_ptr constructor due to C++11 error a83f3c2426

mcelrath renamed this:
~~Warnings clean with C++11~~
Add explicit shared_ptr constructor due to C++11 error
on Oct 29, 2015

laanwj commented at 10:56 AM on October 29, 2015: member

yes, definitely just fix the error. If the warnings point at critical problems, then hiding the warnings by fuzzing the types a bit is a disservice.

utACK

dcousens commented at 12:12 PM on October 29, 2015: contributor

reACK

laanwj merged this on Oct 29, 2015

laanwj closed this on Oct 29, 2015

laanwj referenced this in commit b28c229324 on Oct 29, 2015

mcelrath commented at 12:55 PM on October 29, 2015: none

FWIW, the unsigned change was all to do with the "n" parameter of a Bloom filter, which is its number of bits, and is nonsensical for it to be negative. I'm sure these warnings have nothing to do with C++11 mode...

mcelrath deleted the branch on Oct 29, 2015

zkbot referenced this in commit b0afc4ba1d on Mar 22, 2017

zkbot referenced this in commit f9f48667be on Mar 25, 2017

MarcoFalke locked this on Sep 8, 2021