Make objects in range declarations immutable by default. Avoid unnecessary copying of objects in range declarations.
AbandonedPublic
Actions

Authored by fpelliccioni on Oct 1 2019, 16:32.

Details

Reviewers

Fabien
deadalnix

Group Reviewers

Restricted Project

Summary

Make objects in range declarations immutable by default.

Backport of Bitcoin Core PR13249
https://github.com/bitcoin/bitcoin/pull/13249

Test Plan

make check-all

Diff Detail

Repository

rABC Bitcoin ABC

Branch

feature-backport-5c24d3b98

Lint

Lint OK

Unit

No Unit Test Coverage

Build Status

Buildable 7702
Build 13443: Bitcoin ABC Buildbot (legacy)
Build 13442: arc lint + arc unit

Event Timeline

fpelliccioni created this revision.Oct 1 2019, 16:32

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 1 2019, 16:32

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Diff Testing started.

Harbormaster completed remote builds in B7649: Diff 13293.Oct 1 2019, 16:50

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Part of the PR are missing.

This revision now requires changes to proceed.Oct 1 2019, 18:46

In D4191#100495, @deadalnix wrote:

Part of the PR are missing.

It is a partial application of the PR, I forgot to clarify it, sorry. (summary edited).
I didn't apply the rule for built-in types, because it makes no sense:
for (int x : seq)
is preferred over
for (int const& x : seq).
Have I missed anything else?

fpelliccioni edited the summary of this revision. (Show Details)Oct 3 2019, 13:19

update the summary to clarify that it is a partial application of the PR.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7678: Diff 13335.Oct 3 2019, 13:38

In D4191#100826, @fpelliccioni wrote:

In D4191#100495, @deadalnix wrote:

Part of the PR are missing.

It is a partial application of the PR, I forgot to clarify it, sorry. (summary edited).
I didn't apply the rule for built-in types, because it makes no sense:
for (int x : seq)
is preferred over
for (int const& x : seq).
Have I missed anything else?

for (const int x : seq) is even better. There are still some missing, for instance in qt/bitcoingui.cpp .

This revision now requires changes to proceed.Oct 3 2019, 15:41

In D4191#100850, @deadalnix wrote:

In D4191#100826, @fpelliccioni wrote:

In D4191#100495, @deadalnix wrote:

Part of the PR are missing.

It is a partial application of the PR, I forgot to clarify it, sorry. (summary edited).
I didn't apply the rule for built-in types, because it makes no sense:
for (int x : seq)
is preferred over
for (int const& x : seq).
Have I missed anything else?

for (const int x : seq) is even better. There are still some missing, for instance in qt/bitcoingui.cpp .

I disagree, const is no needed in this case, but... you are the boss ;P

fpelliccioni mentioned this in D4219: Make objects in range declarations immutable by default. Avoid unnecessary copying of objects in range declarations..Oct 7 2019, 14:20

Applying const T x : seq where T is a built-in type.

Build Bitcoin-ABC / Diffs / Diff Testing started.

fpelliccioni edited the summary of this revision. (Show Details)Oct 7 2019, 14:24

Build Bitcoin-ABC / Diffs / Diff Testing started.

Harbormaster completed remote builds in B7702: Diff 13377.Oct 7 2019, 14:35

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Was superseded by D4221.

An error occurred using arc and` git` that I could not solve.
I had help from Fabien.

fpelliccioni mentioned this in D4221: Make objects in range declarations immutable by default. Avoid unnecessary copying of objects in range declarations..Oct 7 2019, 17:52

fpelliccioni mentioned this in rABC33322f23b3d4: Make objects in range declarations immutable by default. Avoid unnecessary….Oct 8 2019, 16:08

fpelliccioni mentioned this in rSTAGING33322f23b3d4: Make objects in range declarations immutable by default. Avoid unnecessary….Oct 9 2019, 20:12

Revision Contents
Changeset List

			Path	Packages
M			src/cuckoocache.h (6 lines)
M			src/qt/bitcoingui.cpp (2 lines)
M			src/qt/sendcoinsdialog.cpp (2 lines)
M			src/qt/trafficgraphwidget.cpp (4 lines)
M			src/qt/transactiondesc.cpp (4 lines)
M			src/qt/transactionrecord.cpp (4 lines)
M			src/test/coins_tests.cpp (10 lines)
M			src/test/dbwrapper_tests.cpp (8 lines)
M			src/test/skiplist_tests.cpp (3 lines)
M			src/test/util_tests.cpp (4 lines)
M			src/timedata.cpp (4 lines)
M			src/validation.cpp (2 lines)
M			src/wallet/rpcdump.cpp (2 lines)
M			src/wallet/wallet.cpp (6 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	13293	93f8083		Oct 1 2019, 16:30	★	★
Diff 2	13335	93f8083	update the summary to clarify that it is a partial application of the PR.	Oct 3 2019, 13:24	★	★
Diff 3	13377	ee1fed5	Applying `const T x : seq` where T is a built-in type.	Oct 7 2019, 14:23	★	★

Commit	Tree	Parents	Author	Summary	Date
d4c28862383a	28c7eeb85d21	ee1fed5258fa	practicalswift	Make objects in range declarations immutable by default. Avoid unnecessary… (Show More…)	Jun 18 2018, 05:58