tests: Add std::to_string to list of locale dependent functions #17851

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:locale-dependence-of-to_string changing 1 files +17 −0

practicalswift commented at 9:01 PM on January 2, 2020: contributor

Add std::to_string to list of locale dependent functions:

std::to_string relies on the current locale for formatting purposes […]

Context #17808 (comment)
tests: Add std::to_string to list of locale dependent functions 1f0adb3dac
fanquake added the label Tests on Jan 2, 2020
MarcoFalke commented at 10:13 PM on January 2, 2020: member

ACK
hebasto approved
hebasto commented at 10:58 PM on January 2, 2020: member

ACK 1f0adb3dac461b3f24f2a98b736d972e9713d67e, I have reviewed the code and it looks OK, I agree it can be merged.
DrahtBot commented at 2:56 AM on January 3, 2020: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
in test/lint/lint-locale-dependence.sh:16 in 1f0adb3dac
```
  11 |      "src/init.cpp:.*atoi"
  12 | +    "src/qt/optionsmodel.cpp.*std::to_string"
  13 |      "src/qt/rpcconsole.cpp:.*atoi"
  14 |      "src/rest.cpp:.*strtol"
  15 | +    "src/rpc/net.cpp.*std::to_string"
  16 | +    "src/rpc/rawtransaction.cpp.*std::to_string"
```
luke-jr commented at 7:34 PM on January 3, 2020:

This shouldn't be an exception, it's a bug?

practicalswift commented at 9:24 PM on January 3, 2020:

I'm not sure I follow: how is this a bug?

Do you mean the inclusion of src/rpc/rawtransaction.cpp in this list, or do you mean the usage of std::to_string in src/rpc/rawtransaction.cpp?

Please clarify :)

Applies also to the cases below.

sipa commented at 6:07 AM on January 4, 2020:

I believe @luke-jr means that the current usage of that function in rpc/rawtransaction.cpp is a bug, and thus should not be silenced, but fixed.

fanquake commented at 6:27 AM on January 4, 2020:

Opened an issue in #17866 so we don't lose track of this.

practicalswift commented at 10:29 AM on January 4, 2020:

@sipa Ah, got it. FWIW I consider all entries in KNOWN_VIOLATIONS to be potential bugs :)

in test/lint/lint-locale-dependence.sh:17 in 1f0adb3dac

  12 | +    "src/qt/optionsmodel.cpp.*std::to_string"
  13 |      "src/qt/rpcconsole.cpp:.*atoi"
  14 |      "src/rest.cpp:.*strtol"
  15 | +    "src/rpc/net.cpp.*std::to_string"
  16 | +    "src/rpc/rawtransaction.cpp.*std::to_string"
  17 | +    "src/rpc/util.cpp.*std::to_string"

luke-jr commented at 7:35 PM on January 3, 2020:

I think this one is a bug too - we don't want RPC help locale-dependent, do we?

in test/lint/lint-locale-dependence.sh:18 in 1f0adb3dac

  13 |      "src/qt/rpcconsole.cpp:.*atoi"
  14 |      "src/rest.cpp:.*strtol"
  15 | +    "src/rpc/net.cpp.*std::to_string"
  16 | +    "src/rpc/rawtransaction.cpp.*std::to_string"
  17 | +    "src/rpc/util.cpp.*std::to_string"
  18 | +    "src/test/addrman_tests.cpp.*std::to_string"

luke-jr commented at 7:36 PM on January 3, 2020:

Bug

in test/lint/lint-locale-dependence.sh:21 in 1f0adb3dac

  16 | +    "src/rpc/rawtransaction.cpp.*std::to_string"
  17 | +    "src/rpc/util.cpp.*std::to_string"
  18 | +    "src/test/addrman_tests.cpp.*std::to_string"
  19 | +    "src/test/blockchain_tests.cpp.*std::to_string"
  20 |      "src/test/dbwrapper_tests.cpp:.*snprintf"
  21 | +    "src/test/denialofservice_tests.cpp.*std::to_string"

luke-jr commented at 7:59 PM on January 3, 2020:

Bug? Or maybe we should be accepting locale numbers in args...

in test/lint/lint-locale-dependence.sh:24 in 1f0adb3dac

  19 | +    "src/test/blockchain_tests.cpp.*std::to_string"
  20 |      "src/test/dbwrapper_tests.cpp:.*snprintf"
  21 | +    "src/test/denialofservice_tests.cpp.*std::to_string"
  22 |      "src/test/fuzz/parse_numbers.cpp:.*atoi"
  23 | +    "src/test/key_tests.cpp.*std::to_string"
  24 | +    "src/test/net_tests.cpp.*std::to_string"

luke-jr commented at 7:59 PM on January 3, 2020:

bug

luke-jr changes_requested
MarcoFalke commented at 8:43 PM on January 3, 2020: member

Going to push this doc/test-only change. Bug fixes can be solved in a separate pull request.
MarcoFalke referenced this in commit 816464198c on Jan 3, 2020
MarcoFalke merged this on Jan 3, 2020
MarcoFalke closed this on Jan 3, 2020
sidhujag referenced this in commit c053415b7c on Jan 4, 2020
sidhujag referenced this in commit a15cf46f3b on Nov 10, 2020
Fabcien referenced this in commit 47a5b8aad9 on Jan 12, 2021
practicalswift deleted the branch on Apr 10, 2021
kittywhiskers referenced this in commit 163600a90d on Feb 27, 2022
kittywhiskers referenced this in commit 7d20efc59d on Feb 27, 2022
kittywhiskers referenced this in commit 6da3dd254f on Feb 28, 2022
kittywhiskers referenced this in commit edf88e133b on Feb 28, 2022
kittywhiskers referenced this in commit 97f49d594b on Feb 28, 2022
kittywhiskers referenced this in commit df13679e8b on Feb 28, 2022
kittywhiskers referenced this in commit 8eb479766f on Mar 13, 2022
kittywhiskers referenced this in commit 17dd1a0fd1 on Mar 24, 2022
kittywhiskers referenced this in commit 38fd597f59 on Mar 24, 2022
DrahtBot locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#17808 util: Replace itostr/i64tostr with c++11 to_string #17866 check std::to_string usage

tests: Add std::to_string to list of locale dependent functions #17851

Conflicts