Skip to content

fuzz: silence a compiler warning about unused CBanEntry comparator #22557

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
fanquake merged 1 commit into from
Jul 28, 2021
Merged

fuzz: silence a compiler warning about unused CBanEntry comparator #22557

fanquake merged 1 commit into from
Jul 28, 2021

Conversation

@vasild
Copy link
Contributor

@vasild vasild commented Jul 26, 2021 

test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
            ^
1 warning generated.

See #22517 (comment)

@vasild vasild mentioned this pull request Jul 26, 2021
Merged
@DrahtBot DrahtBot added the Tests label Jul 26, 2021
@hebasto
Copy link
Member

hebasto commented Jul 26, 2021

See #22517 (comment)

Actually, I mentioned it expecting no one will silent a temporary warning 😄

jarolrod
jarolrod reviewed Jul 26, 2021
View reviewed changes
Copy link
Contributor

@jarolrod jarolrod left a comment •
edited
Loading

Choose a reason for hiding this comment

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

No strong opinion on this. It will be interesting to see the response to this.

Do we want to have a policy of silencing temporary warnings when their is a clear plan that will silence them? --- this warning will be silenced when the fix for the banman fuzz test is implemented.

On the other hand, the argument could be made that #22517 should not have allowed this warning to come up and this PR can be seen as a follow-up to that PR. A followup which finishes properly temporarily disabling the assert.

Tested that this silences the warning on Ubuntu 20.04:
master:

$ make 2>&1 > /dev/null | grep -o "Wunused-function" | wc -l

1

pr:

$ make 2>&1 > /dev/null | grep -o "Wunused-function" | wc -l

0

@vasild
Copy link
Contributor Author

vasild commented Jul 27, 2021

This warning disturbs developers' workflow (compilation with -Werror fails).

It does not matter whether it is temporary, semi-temporary, permanently-temporary, temo-low or should just be painted in purple color.

@maflcko
Copy link
Member

maflcko commented Jul 27, 2021

cr ACK 83dd35260b30eaedc8822326f75c037d5e51f883

Sorry didn't see the warning in my patch

maflcko
maflcko reviewed Jul 27, 2021
View reviewed changes
@vasild
fuzz: silence a compiler warning about unused CBanEntry comparator
787296e 
```
test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
            ^
1 warning generated.
```
@vasild vasild force-pushed the hide_ban_entry_compare branch from 83dd352 to 787296e Compare July 27, 2021 11:59
@vasild
Copy link
Contributor Author

vasild commented Jul 27, 2021

83dd35260b...787296eb67: take review suggestion

hebasto
hebasto approved these changes Jul 27, 2021
View reviewed changes
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 787296e

@laanwj
Copy link
Member

laanwj commented Jul 27, 2021 •
edited
Loading

I don't think we need to follow every temporary change with a change to clean up warnings. If there is intent to use this function soon, I do not see the point of merging an intermediate PR that removes it just to be reintroduced.

Edit: oh, you didn't remove it. This seems fine to me (though still I don't think we should be too busybody about this).

@maflcko
Copy link
Member

maflcko commented Jul 27, 2021

cr ACK 787296e

again my fault for not noticing the compiler warning in the initial patch

@practicalswift
Copy link
Contributor

practicalswift commented Jul 27, 2021

cr ACK 787296e

@fanquake fanquake merged commit be175ce into bitcoin:master Jul 28, 2021
@vasild vasild deleted the hide_ban_entry_compare branch July 28, 2021 09:12
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 28, 2021
@fanquake
Merge bitcoin#22557: fuzz: silence a compiler warning about unused CB…
2870633 
…anEntry comparator

787296e fuzz: silence a compiler warning about unused CBanEntry comparator (Vasil Dimov)

Pull request description:

  ```
  test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
  static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
              ^
  1 warning generated.
  ```

  See bitcoin#22517 (comment)

ACKs for top commit:
  MarcoFalke:
    cr ACK 787296e
  practicalswift:
    cr ACK 787296e
  hebasto:
    ACK 787296e

Tree-SHA512: 72e483cef249170160879cf4b69b787fb6c539d61dda423f618e2c5f130bee8c42897487751e5b58e7679cdb0153eb80efcb104e8a85095daa60d47e39ce78b8
@hebasto hebasto mentioned this pull request Aug 6, 2021
Merged
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 6, 2021
@vasild @hebasto
fuzz: silence a compiler warning about unused CBanEntry comparator
9214541 
```
test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
            ^
1 warning generated.
```

Github-Pull: bitcoin#22557
Rebased-From: 787296e
@hebasto
Copy link
Member

hebasto commented Aug 6, 2021 •
edited
Loading

Backported in #22629.

UPDATE: #22629 (comment)

hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 6, 2021
@vasild @hebasto
fuzz: silence a compiler warning about unused CBanEntry comparator
482ab6d 
```
test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
            ^
1 warning generated.
```

Github-Pull: bitcoin#22557
Rebased-From: 787296e
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 7, 2021
@vasild @hebasto
fuzz: silence a compiler warning about unused CBanEntry comparator
20c1331 
```
test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
            ^
1 warning generated.
```

Github-Pull: bitcoin#22557
Rebased-From: 787296e
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 9, 2021
@vasild @hebasto
fuzz: silence a compiler warning about unused CBanEntry comparator
c427a01 
```
test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
            ^
1 warning generated.
```

Github-Pull: bitcoin#22557
Rebased-From: 787296e
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 9, 2021
@vasild @hebasto
fuzz: silence a compiler warning about unused CBanEntry comparator
b5968df 
```
test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
            ^
1 warning generated.
```

Github-Pull: bitcoin#22557
Rebased-From: 787296e
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 20, 2021
@vasild @hebasto
fuzz: silence a compiler warning about unused CBanEntry comparator
e3d207d 
```
test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
            ^
1 warning generated.
```

Github-Pull: bitcoin#22557
Rebased-From: 787296e
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Aug 20, 2021
@vasild @hebasto
fuzz: silence a compiler warning about unused CBanEntry comparator
f6c4c79 
```
test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
            ^
1 warning generated.
```

Github-Pull: bitcoin#22557
Rebased-From: 787296e
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto approved these changes

+2 more reviewers

@maflcko maflcko maflcko left review comments

@jarolrod jarolrod jarolrod left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@vasild @hebasto @maflcko @laanwj @practicalswift @jarolrod @fanquake @DrahtBot