Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
laanwj merged 1 commit into from
Oct 29, 2015
Merged

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

laanwj merged 1 commit into from
Oct 29, 2015

Conversation

@mcelrath
Copy link

@mcelrath mcelrath commented Oct 28, 2015

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
Copy link
Contributor

dcousens commented Oct 29, 2015

utACK

dcousens
dcousens reviewed Oct 29, 2015
View reviewed changes
src/leveldb/include/leveldb/filter_policy.h Outdated
Copy link
Contributor

@dcousens dcousens Oct 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@gmaxwell
Copy link
Contributor

gmaxwell commented Oct 29, 2015

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

@jgarzik
Copy link
Contributor

jgarzik commented Oct 29, 2015

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

@dcousens
Copy link
Contributor

dcousens commented Oct 29, 2015

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

@mcelrath
Copy link
Author

mcelrath commented Oct 29, 2015

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.

@mcelrath mcelrath changed the title Warnings clean with C++11 Add explicit shared_ptr constructor due to C++11 error Oct 29, 2015
@laanwj
Copy link
Member

laanwj commented Oct 29, 2015

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
Copy link
Contributor

dcousens commented Oct 29, 2015

reACK

@laanwj laanwj merged commit a83f3c2 into bitcoin:master Oct 29, 2015
laanwj added a commit that referenced this pull request Oct 29, 2015
@laanwj
Merge pull request #6899
b28c229 
a83f3c2 Add explicit shared_ptr constructor due to C++11 error (Bob McElrath)
@mcelrath
Copy link
Author

mcelrath commented Oct 29, 2015

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 mcelrath deleted the cpp11 branch October 29, 2015 14:36
@str4d str4d mentioned this pull request Mar 14, 2017
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Mar 22, 2017
@zkbot
Auto merge of #2176 - str4d:1593-libevent, r=<try>
b0afc4b 
libevent-based http server

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#5677
- bitcoin/bitcoin#6695
- bitcoin/bitcoin#6899
- bitcoin/bitcoin#7016
- bitcoin/bitcoin#7964
- bitcoin/bitcoin#8722
- bitcoin/bitcoin#8730
- bitcoin/bitcoin#9073
- bitcoin/bitcoin#9265
- bitcoin/bitcoin#9387
- bitcoin/bitcoin#9471
- bitcoin/bitcoin#9647
- bitcoin/bitcoin#9903

Closes #1593 and #1856.
zkbot added a commit to zcash/zcash that referenced this pull request Mar 25, 2017
@zkbot
Auto merge of #2176 - str4d:1593-libevent, r=bitcartel
f9f4866 
libevent-based http server

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#5677
- bitcoin/bitcoin#6695
- bitcoin/bitcoin#6899
- bitcoin/bitcoin#7016
- bitcoin/bitcoin#7964
- bitcoin/bitcoin#8722
- bitcoin/bitcoin#8730
- bitcoin/bitcoin#9073
- bitcoin/bitcoin#9265
- bitcoin/bitcoin#9387
- bitcoin/bitcoin#9471
- bitcoin/bitcoin#9647
- bitcoin/bitcoin#9903
- bitcoin/bitcoin#6640
- bitcoin/bitcoin#8139
- bitcoin/bitcoin#8839

Closes #1593 and #1856.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mcelrath @dcousens @gmaxwell @jgarzik @laanwj