Page MenuHomePhabricator

Remove CBlockIndex::SetNull
ClosedPublic

Authored by deadalnix on May 29 2020, 13:09.

Details

Reviewers
majcosta
Group Reviewers
Restricted Project
Commits
rABC89778bcdb086: Remove CBlockIndex::SetNull
Summary

Replace it with proper initializer use.

Test Plan
ninja all check-all

Diff Detail

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

Event Timeline

majcosta added inline comments.
src/chain.h
55 ↗(On Diff #20698)

although for these PODs it won't matter, consider using direct list initialization, since in-class assignment might make copies

113 ↗(On Diff #20698)

no need to delegate to CBlockIndex() since it doesn't do anything. Rather you could also use an initializer list instead of assignment in the constructor body.

src/chain.h
55 ↗(On Diff #20698)

I'd rather ensure that the types are movable if need be.

113 ↗(On Diff #20698)

Keeping the code similar to upstream is a feature, IMO. Plus I'm fairly confident the codegen won't be any different considering this is just copying a few ints around under the hood.

jasonbcox added inline comments.
src/chain.h
55 ↗(On Diff #20698)

TIL initializer lists aren't movable :/

Is it possible to statically assert that CBlockIndex IS movable? Otherwise regressing on this to match upstream appears imminent.

This revision is now accepted and ready to land.May 29 2020, 22:29
This revision was automatically updated to reflect the committed changes.

This is very similar to PR17162. Might be useful to add the reference to the revision description.