Details

Reviewers

jasonbcox
deadalnix

Group Reviewers

Restricted Project

Commits

rABC52379b02de1e: [backport#14599] Use functions guaranteed to be locale independent

Summary

Use IsDigit(...) instead of std::isdigit
Use ToLower(...) instead of std::tolower
Use IsSpace(...) instead of boost::is_space
Update KNOWN_VIOLATIONS: Remove fixed violations

Backport of Core PR14599

Test Plan

ninja check

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 11085
Build 20612: Default Diff Build & Tests
Build 20611: Build without Wallet
Build 20610: Build with clang-10
Build 20609: arc lint + arc unit

Event Timeline

majcosta created this revision.May 29 2020, 17:30

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 29 2020, 17:30

majcosta requested review of this revision.May 29 2020, 17:30

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

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

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

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

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

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

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

Harbormaster completed remote builds in B11085: Diff 20706.May 29 2020, 17:40

majcosta planned changes to this revision.May 29 2020, 17:49

reverted some core changes to locale-dependence linter that were wrong

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

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

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

majcosta added a child revision: D6295: [backport#15139] util: Make ToLower and ToUpper take a char.May 29 2020, 18:04

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

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

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

Harbormaster completed remote builds in B11086: Diff 20707.May 29 2020, 18:13

Are the missed changes in LocaleDependenceLinter due to missing backports? If so, why not do those first?

src/uint256.cpp
33 ↗	(On Diff #20707)	cast should be function style: `uint8_t(...)`

This revision now requires changes to proceed.May 29 2020, 20:26

deadalnix requested changes to this revision.May 29 2020, 21:16

deadalnix added a subscriber: deadalnix.

deadalnix added inline comments.

src/uint256.cpp
33	C++ constructor instead of C cast

In D6294#155360, @jasonbcox wrote:

Are the missed changes in LocaleDependenceLinter due to missing backports? If so, why not do those first?

I've looked for them, but didn't find them. Looks like a legit mistake or they reintroduced them later. They're calling strtoll and strtoull in util/strencodings.cpp today.

replaced C-style cast

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

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

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

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

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

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

Harbormaster completed remote builds in B11103: Diff 20731.May 30 2020, 02:45

deadalnix requested changes to this revision.May 31 2020, 22:01

deadalnix added inline comments.

src/uint256.cpp
33 ↗	(On Diff #20731)	You did change the behavior here, and it's fishy tests are passing. You might want to use that opportunity to add a handful of test cases.