Skip to content
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

Drop dead code CScript::Find #12969

Merged
merged 1 commit into from
Apr 13, 2018
Merged

Drop dead code CScript::Find #12969

merged 1 commit into from
Apr 13, 2018

Conversation

Empact
Copy link
Member

@Empact Empact commented Apr 12, 2018 •
edited
Loading

Last use removed in 922e8e2 (2012!)

@Empact Empact mentioned this pull request Apr 12, 2018
Merged
@Empact
Copy link
Member Author

Empact commented Apr 12, 2018

Do we have a mechanism for dead code detection?

@maflcko
Copy link
Member

maflcko commented Apr 12, 2018

Please include in what commit it was last used.

@Empact
Copy link
Member Author

Empact commented Apr 12, 2018

@MarcoFalke 922e8e2 (2012!)

@jimpo
Copy link
Contributor

jimpo commented Apr 13, 2018

utACK c6d4a81.

grepped codebase for Find(. Other instances found are CAddrMan::Find and FindFileTest::Find in leveldb.

@Empact
Copy link
Member Author

Empact commented Apr 13, 2018

The test failure is known issue #12863

@Empact
Drop dead code CScript::Find
8cbc5c4 
Last use removed in 922e8e2 (2012!)
@Empact
Copy link
Member Author

Empact commented Apr 13, 2018

Rebased for CI

@ajtowns
Copy link
Contributor

ajtowns commented Apr 13, 2018

NACK 8cbc5c4 -- code is fine, but it introduces a merge conflict with #12885 due to being adjacent in the file being patched, so this should be rebased after that is merged IMO.

@practicalswift
Copy link
Contributor

practicalswift commented Apr 13, 2018 •
edited
Loading

utACK 8cbc5c4

I don't see any Find(…) usage being removed in 922e8e2?

My reading is that Find(…) has been unused since being introduced in e679ec9.

Anyways – nice find!

Edit: Found the removed usage. GitHub's interface tricked me :-)

@ajtowns
Copy link
Contributor

ajtowns commented Apr 13, 2018 •
edited
Loading

@practicalswift There's subscript.Find(OP_CODESEPERATOR), scriptPubKey.Find(OP_EVAL) and scriptSig.Find(OP_EVAL) removed in that commit. Github hides them with a "Large diffs are not rendered by default." though.

@laanwj
Copy link
Member

laanwj commented Apr 13, 2018

drop dead code 🐭
utACK 8cbc5c4

@sipa
Copy link
Member

sipa commented Apr 13, 2018 •
edited
Loading

Drop dead, code! utACK 8cbc5c4

@ajtowns I'll gladly rebase #12885 if this is merged first.

@Empact
Copy link
Member Author

Empact commented Apr 13, 2018

@sipa likewise on this side :)

@promag
Copy link
Member

promag commented Apr 13, 2018

@ajtowns sorry you mean @practicalswift?

@promag
Copy link
Member

promag commented Apr 13, 2018

utACK 8cbc5c4.

@laanwj laanwj merged commit 8cbc5c4 into bitcoin:master Apr 13, 2018
laanwj added a commit that referenced this pull request Apr 13, 2018
@laanwj
Merge #12969: Drop dead code CScript::Find
94deb09 
8cbc5c4 Drop dead code CScript::Find (Ben Woosley)

Pull request description:

  Last use removed in 922e8e2 (2012!)

Tree-SHA512: d62daf2fc340a04c5f68990aa450d6738fadf4df6c50507fe02ecb62ae329b77db401719bf556a69f747408efdea5bc98af5e41ba82650b2f6d8b2d5489625e4
@Empact Empact deleted the cscript-find branch April 17, 2018 09:21
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Aug 16, 2019
@laanwj
Merge #12969: Drop dead code CScript::Find
cdbd298 
Summary:
8cbc5c4 Drop dead code CScript::Find (Ben Woosley)

Pull request description:

  Last use removed in 922e8e2 (2012!)

Tree-SHA512: d62daf2fc340a04c5f68990aa450d6738fadf4df6c50507fe02ecb62ae329b77db401719bf556a69f747408efdea5bc98af5e41ba82650b2f6d8b2d5489625e4

Backport of Core PR12969
bitcoin/bitcoin#12969

Test Plan:
  make check

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: deadalnix, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3850
jonspock pushed a commit to jonspock/devault that referenced this pull request Nov 4, 2019
@laanwj @jonspock
Merge #12969: Drop dead code CScript::Find
a03c73d 
Summary:
8cbc5c4 Drop dead code CScript::Find (Ben Woosley)

Pull request description:

  Last use removed in 922e8e2 (2012!)

Tree-SHA512: d62daf2fc340a04c5f68990aa450d6738fadf4df6c50507fe02ecb62ae329b77db401719bf556a69f747408efdea5bc98af5e41ba82650b2f6d8b2d5489625e4

Backport of Core PR12969
bitcoin/bitcoin#12969

Test Plan:
  make check

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: deadalnix, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3850
proteanx pushed a commit to devaultcrypto/devault that referenced this pull request Nov 4, 2019
@laanwj @proteanx
Merge #12969: Drop dead code CScript::Find
147e01b 
Summary:
8cbc5c4 Drop dead code CScript::Find (Ben Woosley)

Pull request description:

  Last use removed in 922e8e2 (2012!)

Tree-SHA512: d62daf2fc340a04c5f68990aa450d6738fadf4df6c50507fe02ecb62ae329b77db401719bf556a69f747408efdea5bc98af5e41ba82650b2f6d8b2d5489625e4

Backport of Core PR12969
bitcoin/bitcoin#12969

Test Plan:
  make check

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: deadalnix, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3850
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 17, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12969: Drop dead code CScript::Find
689a5d2 
8cbc5c4 Drop dead code CScript::Find (Ben Woosley)

Pull request description:

  Last use removed in 922e8e2 (2012!)

Tree-SHA512: d62daf2fc340a04c5f68990aa450d6738fadf4df6c50507fe02ecb62ae329b77db401719bf556a69f747408efdea5bc98af5e41ba82650b2f6d8b2d5489625e4
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 17, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12969: Drop dead code CScript::Find
bd6f399 
8cbc5c4 Drop dead code CScript::Find (Ben Woosley)

Pull request description:

  Last use removed in 922e8e2 (2012!)

Tree-SHA512: d62daf2fc340a04c5f68990aa450d6738fadf4df6c50507fe02ecb62ae329b77db401719bf556a69f747408efdea5bc98af5e41ba82650b2f6d8b2d5489625e4
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 27, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12969: Drop dead code CScript::Find
0036b30 
8cbc5c4 Drop dead code CScript::Find (Ben Woosley)

Pull request description:

  Last use removed in 922e8e2 (2012!)

Tree-SHA512: d62daf2fc340a04c5f68990aa450d6738fadf4df6c50507fe02ecb62ae329b77db401719bf556a69f747408efdea5bc98af5e41ba82650b2f6d8b2d5489625e4
@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
Refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@Empact @maflcko @jimpo @ajtowns @practicalswift @laanwj @sipa @promag @fanquake