Page MenuHomePhabricator
Differential D5470

Add << operator overload for PeerMessagingState
ClosedPublic
Actions

Authored by nakihito on Mar 12 2020, 20:49.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Maniphest Tasks
T762: Build Bitcoin-ABC / Master / Bitcoin-ABC Master ASAN is broken.
T763: Build Bitcoin-ABC / Master / Bitcoin-ABC Master UBSAN is broken.
T764: Build Bitcoin-ABC / Resource Intensive Builds / Bitcoin-ABC Master TSAN is broken.
Commits
rSTAGINGf8b0f9874998: Add << operator overload for PeerMessagingState
rABCf8b0f9874998: Add << operator overload for PeerMessagingState
Summary

Not having this causes BOOST_CHECK_EQUAL() to fail on sanitizer builds when comparing
the enum.

Fixes T764, T763, and T762

Test Plan
ninja check

Run sanitizer builds

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

nakihito created this revision.Mar 12 2020, 20:49
Owners added a reviewer: Restricted Owners Package.Mar 12 2020, 20:49
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 12 2020, 20:49
nakihito planned changes to this revision.Mar 12 2020, 20:49
teamcity added a comment.Mar 12 2020, 20:49

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

nakihito edited the summary of this revision. (Show Details)Mar 12 2020, 20:50
nakihito added tasks: T764: Build Bitcoin-ABC / Resource Intensive Builds / Bitcoin-ABC Master TSAN is broken., T763: Build Bitcoin-ABC / Master / Bitcoin-ABC Master UBSAN is broken., T762: Build Bitcoin-ABC / Master / Bitcoin-ABC Master ASAN is broken..
teamcity added a comment.Mar 12 2020, 20:51

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) started.

teamcity added a comment.Mar 12 2020, 20:52

Build Bitcoin-ABC / Diffs / Diff Testing (build-ubsan) started.

teamcity added a comment.Mar 12 2020, 20:53

Build Bitcoin-ABC / Diffs (Resource Intensive) / Resource Intensive Diff Testing (build-tsan) started.

teamcity added a comment.Mar 12 2020, 21:00

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B9800: Diff 16908.Mar 12 2020, 21:00
teamcity added a comment.Mar 12 2020, 21:06

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) passed.

teamcity added a comment.Mar 12 2020, 21:25

Build Bitcoin-ABC / Diffs / Diff Testing (build-ubsan) passed.

nakihito requested review of this revision.Mar 12 2020, 21:27
teamcity added a comment.Mar 12 2020, 21:27

Build Bitcoin-ABC / Diffs (Resource Intensive) / Resource Intensive Diff Testing (build-tsan) passed.

jasonbcox mentioned this in D5471: Add enum for parse_name() return value.Mar 13 2020, 04:06
deadalnix requested changes to this revision.Mar 13 2020, 07:29
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/seeder/bitcoin.cpp
23 ↗(On Diff #16908)

IMO you should separate the C++ magic from the intent of the code, because right now, that is not very clear.

For instance, you could use something like:

template<typename E>
constexpr std::underlying_type<E>::type to_integral(E e) {
   return static_cast<typename std::underlying_type<E>::type>(e);
}

And then call that. If it turns out to be more generally useful, then it can be moved to a header somewhere.

src/seeder/bitcoin.h
12 ↗(On Diff #16908)

ostream only is suffiscient.

14 ↗(On Diff #16908)

Remove.

This revision now requires changes to proceed.Mar 13 2020, 07:29
nakihito updated this revision to Diff 16918.Mar 13 2020, 17:12

Separated C++ magic from operator function as advised.

nakihito planned changes to this revision.Mar 13 2020, 17:12
teamcity added a comment.Mar 13 2020, 17:13

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.Mar 13 2020, 17:13

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) started.

nakihito updated this revision to Diff 16920.Mar 13 2020, 17:21

Removed ostream from seeder/bitcoin.h.

teamcity added a comment.Mar 13 2020, 17:21

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.Mar 13 2020, 17:22

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) started.

teamcity added a comment.Mar 13 2020, 17:24

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B9805: Diff 16918.Mar 13 2020, 17:24
teamcity added a comment.Mar 13 2020, 17:28

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) passed.

teamcity added a comment.Mar 13 2020, 17:31

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B9807: Diff 16920.Mar 13 2020, 17:31
jasonbcox added a subscriber: jasonbcox.Mar 13 2020, 17:41
jasonbcox added inline comments.
src/seeder/bitcoin.h
25 ↗(On Diff #16920)

Nit: Same feedback as https://reviews.bitcoinabc.org/D5471#inline-33727

teamcity added a comment.Mar 13 2020, 17:47

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) passed.

nakihito updated this revision to Diff 16923.Mar 13 2020, 18:55

Removed unnecessary underlying value.

teamcity added a comment.Mar 13 2020, 18:55

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.Mar 13 2020, 19:05

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B9809: Diff 16923.Mar 13 2020, 19:05
teamcity added a comment.Mar 13 2020, 19:27

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) started.

teamcity added a comment.Mar 13 2020, 19:42

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) passed.

nakihito updated this revision to Diff 16927.Mar 13 2020, 20:17

Moved to_integral() to a new file. because it is only really used for tests. This makes it easier to include in all (appropriate) test files without having to expose or open up more components.

teamcity added a comment.Mar 13 2020, 20:18

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.Mar 13 2020, 20:19

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) started.

teamcity added a comment.Mar 13 2020, 20:27

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B9812: Diff 16927.Mar 13 2020, 20:27
teamcity added a comment.Mar 13 2020, 20:33

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) passed.

nakihito added a child revision: D5471: Add enum for parse_name() return value.Mar 13 2020, 20:45
jasonbcox added inline comments.Mar 13 2020, 23:49
src/seeder/bitcoin.h
22 ↗(On Diff #16927)

You missed the bool here

nakihito updated this revision to Diff 16936.Mar 14 2020, 01:01

Removed underlying bool type.

teamcity added a comment.Mar 14 2020, 01:02

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.Mar 14 2020, 01:02

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) started.

teamcity added a comment.Mar 14 2020, 01:11

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B9818: Diff 16936.Mar 14 2020, 01:11
teamcity added a comment.Mar 14 2020, 01:23

Build Bitcoin-ABC / Diffs / Diff Testing (build-asan) passed.

deadalnix accepted this revision.Mar 14 2020, 10:28
This revision is now accepted and ready to land.Mar 14 2020, 10:28
Closed by commit rABCf8b0f9874998: Add << operator overload for PeerMessagingState (authored by nakihito). · Explain WhyMar 14 2020, 16:26
This revision was automatically updated to reflect the committed changes.
nakihito added a commit: rABCf8b0f9874998: Add << operator overload for PeerMessagingState.
nakihito added a commit: rSTAGINGf8b0f9874998: Add << operator overload for PeerMessagingState.Apr 7 2020, 23:20

Revision Contents
Changeset List

PathSize
src/
seeder/
6 lines
test/
7 lines
15 lines

Diff 16937

View Options

src/seeder/bitcoin.h

Loading...
View Options

src/seeder/test/p2p_messaging_tests.cpp

Loading...
View Options

src/seeder/test/util.h

Loading...