Skip to content

Conversation

fanquake
Copy link
Member

boost::bind usage was removed in #13743. However a new usage snuck in as
part of 2bc4c3e (#15225).

boost::bind usage was removed in bitcoin#13743. However a new usage snuck in as
part of 2bc4c3e (bitcoin#15225).
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK c4be50f, it seems a linter is required :)

@DrahtBot DrahtBot added the GUI label Aug 31, 2020
@practicalswift
Copy link
Contributor

ACK c4be50f -- diff looks correct

@hebasto

ACK c4be50f, it seems a linter required :)

Linter requested. Linter delivered:

new file mode 100755
index 000000000..99d60b58d
--- /dev/null
+++ b/test/lint/lint-cpp.sh
@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2018-2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+#
+# Check for various C++ code patterns we want to avoid.
+
+export LC_ALL=C
+
+EXIT_CODE=0
+
+OUTPUT=$(git grep -E "boost::bind\(" -- "*.cpp" "*.h")
+if [[ ${OUTPUT} != "" ]]; then
+    echo "Use of boost::bind detected. Use std::bind instead."
+    echo
+    echo "${OUTPUT}"
+    EXIT_CODE=1
+fi
+
+exit ${EXIT_CODE}

Always happy to lint!

@laanwj
Copy link
Member

laanwj commented Aug 31, 2020

ACK
Thanks @practicalswift — I think we do need a linter here, my first idea was to forbid the header that introduces boost::bind instead, however it seems it's included indirectly through almost anything boost …

@promag
Copy link
Contributor

promag commented Aug 31, 2020

ACK + linter.

This currently only checks for boost::bind usage.

Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
@fanquake
Copy link
Member Author

fanquake commented Sep 1, 2020

C++ linter added as requested.

@practicalswift
Copy link
Contributor

ACK e36f802 -- patch looks correct

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK e36f802

Result of cherry-picking only the linter commit into the master branch:

$ test/lint/lint-cpp.sh
Use of boost::bind detected. Use std::bind instead.

src/qt/walletmodel.cpp:    m_handler_can_get_addrs_changed = m_wallet->handleCanGetAddressesChanged(boost::bind(NotifyCanGetAddressesChanged, this));

EXIT_CODE=1
fi

exit ${EXIT_CODE}
Copy link
Member

Choose a reason for hiding this comment

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

nanonit:
Screenshot from 2020-09-01 10-41-43

@fanquake fanquake merged commit 9876ab8 into bitcoin:master Sep 3, 2020
@fanquake fanquake deleted the sneak_boost_bind branch September 3, 2020 03:45
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Aug 21, 2021
This currently only checks for boost::bind usage.

Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 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.

7 participants