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

developer notes: updates for C++11 #8177

Merged
merged 1 commit into from Jun 10, 2016

Conversation

Projects
None yet
4 participants
@kazcw
Contributor

kazcw commented Jun 8, 2016

No description provided.

@kazcw kazcw changed the title from developer notes: deprecate C-style casts to developer notes: deprecate C-style casts + updates for C++11 Jun 8, 2016

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Jun 8, 2016

Contributor

concept ACK

Contributor

dcousens commented Jun 8, 2016

concept ACK

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jun 8, 2016

Member
Member

sipa commented Jun 8, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 9, 2016

Member

I'm fine with C-style casts too for primitive types. No need to change anything there, they're not more magical than anything else.

For object types and such it makes sense to require c++ casts.

ACK on the scoped_ptr to unique_ptr change.

Member

laanwj commented Jun 9, 2016

I'm fine with C-style casts too for primitive types. No need to change anything there, they're not more magical than anything else.

For object types and such it makes sense to require c++ casts.

ACK on the scoped_ptr to unique_ptr change.

@laanwj laanwj added the Docs label Jun 9, 2016

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Jun 9, 2016

Contributor

@laanwj it does make searching for casts and points of concern much easier if you use static_cast and the rest of the C++ family of casts.
The C-syntax, while more concise in context, often bears a higher intellectual load as to the broader implications of the action in the long run, IMHO.

The extra safety guarantees are also a boon and pretty excellent for what I would consider safety conscious software.

It also makes casts even uglier, as they should be.

Contributor

dcousens commented Jun 9, 2016

@laanwj it does make searching for casts and points of concern much easier if you use static_cast and the rest of the C++ family of casts.
The C-syntax, while more concise in context, often bears a higher intellectual load as to the broader implications of the action in the long run, IMHO.

The extra safety guarantees are also a boon and pretty excellent for what I would consider safety conscious software.

It also makes casts even uglier, as they should be.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 9, 2016

Member

it does make searching for casts and points of concern much easier if you use static_cast and the rest of the C++ family of casts.

How often do you actually want to search for all casts?

The extra safety guarantees are also a boon and pretty excellent for what I would consider safety conscious software.

Can you be more specific here? How are casts of primitive types safer using C++ syntax?

I know that pointer casts are safer using C++ syntax and as they are the dangerous kind of casting. I agree with using C++ syntax when casting from const to non const pointers, or one kind of pointer to another, but I don't agree doing so for all cases where primitive types are converted.

Member

laanwj commented Jun 9, 2016

it does make searching for casts and points of concern much easier if you use static_cast and the rest of the C++ family of casts.

How often do you actually want to search for all casts?

The extra safety guarantees are also a boon and pretty excellent for what I would consider safety conscious software.

Can you be more specific here? How are casts of primitive types safer using C++ syntax?

I know that pointer casts are safer using C++ syntax and as they are the dangerous kind of casting. I agree with using C++ syntax when casting from const to non const pointers, or one kind of pointer to another, but I don't agree doing so for all cases where primitive types are converted.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 9, 2016

Member

I'm also not looking forward to 20 PRs full of 'convert one type of cast to another'. In the time we run this project we've (as far as I know) never had a bug caused by casting. There's no immediate need here. I wrote those developer suggestions all after having found an issue caused by that particular thing. They're not meant for style preferences.

Member

laanwj commented Jun 9, 2016

I'm also not looking forward to 20 PRs full of 'convert one type of cast to another'. In the time we run this project we've (as far as I know) never had a bug caused by casting. There's no immediate need here. I wrote those developer suggestions all after having found an issue caused by that particular thing. They're not meant for style preferences.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 9, 2016

Member

We should mention this one: avoid shadowing vars, and enable -Wshadow #8105
Very sneaky bug introduced by this: #8102 (comment)

Member

laanwj commented Jun 9, 2016

We should mention this one: avoid shadowing vars, and enable -Wshadow #8105
Very sneaky bug introduced by this: #8102 (comment)

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Jun 9, 2016

Contributor

Can you be more specific here? How are casts of primitive types safer using C++ syntax?

I know that pointer casts are safer using C++ syntax and as they are the dangerous kind of casting.

Perhaps I mis-indicated that I was referring only to int style casts, the above was a blanket opinion.

In respect to:

I agree with using C++ syntax when casting from const to non const pointers, or one kind of pointer to another, but I don't agree doing so for all cases where primitive types are converted.

I think this is a good compromise.
That said, IMHO, primitive conversions can often [but certainly not always] be an indication that the underlying type is over-constrained or just plain wrong. But that problem isn't fixed by this change, it is just made more obvious.

In light of:

I'm also not looking forward to 20 PRs full of 'convert one type of cast to another'

I too think this may be just another nit-pick enabling PR, but, it is certainly a desirable goal... at some point.

Contributor

dcousens commented Jun 9, 2016

Can you be more specific here? How are casts of primitive types safer using C++ syntax?

I know that pointer casts are safer using C++ syntax and as they are the dangerous kind of casting.

Perhaps I mis-indicated that I was referring only to int style casts, the above was a blanket opinion.

In respect to:

I agree with using C++ syntax when casting from const to non const pointers, or one kind of pointer to another, but I don't agree doing so for all cases where primitive types are converted.

I think this is a good compromise.
That said, IMHO, primitive conversions can often [but certainly not always] be an indication that the underlying type is over-constrained or just plain wrong. But that problem isn't fixed by this change, it is just made more obvious.

In light of:

I'm also not looking forward to 20 PRs full of 'convert one type of cast to another'

I too think this may be just another nit-pick enabling PR, but, it is certainly a desirable goal... at some point.

developer notes: updates for C++11
- boost::scoped_ptr is obsolete
- std::vector::data replaces begin_ptr / end_ptr
@kazcw

This comment has been minimized.

Show comment
Hide comment
@kazcw

kazcw Jun 9, 2016

Contributor

Removed the statement on C-style casts.

Contributor

kazcw commented Jun 9, 2016

Removed the statement on C-style casts.

@laanwj laanwj changed the title from developer notes: deprecate C-style casts + updates for C++11 to developer notes: updates for C++11 Jun 10, 2016

@laanwj laanwj merged commit 654a211 into bitcoin:master Jun 10, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jun 10, 2016

Merge #8177: developer notes: updates for C++11
654a211 developer notes: updates for C++11 (Kaz Wesley)

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #8177: developer notes: updates for C++11
654a211 developer notes: updates for C++11 (Kaz Wesley)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #8177: developer notes: updates for C++11
654a211 developer notes: updates for C++11 (Kaz Wesley)

codablock added a commit to codablock/dash that referenced this pull request Dec 22, 2017

Merge #8177: developer notes: updates for C++11
654a211 developer notes: updates for C++11 (Kaz Wesley)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment