Skip to content

Conversation

@practicalswift
Copy link
Contributor

Remove obsolete NODISCARD ifdef forest. Use [[nodiscard]] (C++17).

@fanquake
Copy link
Member

Now that the ifdef forest is removed, is there an advantage to redefining [[nodiscard]] as NODISCARD rather than just using [[nodiscard]] directly where applicable?

-BEGIN VERIFY SCRIPT-
sed -i "s/NODISCARD/[[nodiscard]]/g" $(git grep -l "NODISCARD" ":(exclude)src/bench/nanobench.h" ":(exclude)src/attributes.h")
-END VERIFY SCRIPT-
@practicalswift
Copy link
Contributor Author

@fanquake No, not at all: I just wanted to keep the diff down. But you have a good point: using [[nodiscard]] directly is certainly nicer and more idiomatic, so it is probably worth the one time cost of a few extra lines of diff. Now doing that using a scripted-diff :)

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

ACK 79bff8e
Checked that all instances of NODISCARD were tackled.
It's always nice to see when old cruft is removed 👍

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK 79bff8e

@maflcko maflcko merged commit 81d5af4 into bitcoin:master Nov 30, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 30, 2020
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Feb 16, 2021
@practicalswift practicalswift deleted the remove-nodiscard-cruft branch April 10, 2021 19:43
kwvg added a commit to kwvg/dash that referenced this pull request Jun 27, 2021
…discard]] (C++17)

------------- BEGIN SCRIPT ---------------
gsed -i "s/NODISCARD/[[nodiscard]]/g" $(git grep -l "NODISCARD" ":(exclude)src/bench/nanobench.h" ":(exclude)src/attributes.h")
------------- END   SCRIPT ---------------
UdjinM6 added a commit to dashpay/dash that referenced this pull request Jun 27, 2021
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jan 26, 2022
Summary:
> scripted-diff: Use [[nodiscard]] (C++17) instead of NODISCARD
>
> ```
> -BEGIN VERIFY SCRIPT-
> sed -i "s/NODISCARD/[[nodiscard]]/g" $(git grep -l "NODISCARD" ":(exclude)src/bench/nanobench.h" ":(exclude)src/attributes.h")
> -END VERIFY SCRIPT-
> ```

> Remove NODISCARD

This is a backport of [[bitcoin/bitcoin#20499 | core#20499]]
Depends on D10888

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10889
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Apr 29, 2022
…discard]] (C++17)

------------- BEGIN SCRIPT ---------------
gsed -i "s/NODISCARD/[[nodiscard]]/g" $(git grep -l "NODISCARD" ":(exclude)src/bench/nanobench.h" ":(exclude)src/attributes.h")
------------- END   SCRIPT ---------------
maflcko pushed a commit that referenced this pull request May 23, 2022
71a8dbe refactor: Remove defunct attributes.h includes (Ben Woosley)

Pull request description:

  Since the removal of NODISCARD in 81d5af4,
  the only attributes.h def is LIFETIMEBOUND, and it's included in many more
  places that it is used.

  This removes all includes which do not have an associated use of LIFETIMEBOUND,
  and adds it to the following files, due to their use of the same:
  * src/validationinterface.h
  * src/script/standard.h

  See also #20499.

Top commit has no ACKs.

Tree-SHA512: f3e10a5cda5ab78371b77b702f4a241ff69d490a16cc6059f1a4202b97c584accdbc951cc7b6120eae94bee3b9249e9117b45cf6ed1a5228ca23b5638fcf7b7b
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 28, 2022
71a8dbe refactor: Remove defunct attributes.h includes (Ben Woosley)

Pull request description:

  Since the removal of NODISCARD in 81d5af4,
  the only attributes.h def is LIFETIMEBOUND, and it's included in many more
  places that it is used.

  This removes all includes which do not have an associated use of LIFETIMEBOUND,
  and adds it to the following files, due to their use of the same:
  * src/validationinterface.h
  * src/script/standard.h

  See also bitcoin#20499.

Top commit has no ACKs.

Tree-SHA512: f3e10a5cda5ab78371b77b702f4a241ff69d490a16cc6059f1a4202b97c584accdbc951cc7b6120eae94bee3b9249e9117b45cf6ed1a5228ca23b5638fcf7b7b
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants