Renamed UniValue::__pushKV to UniValue::pushKVEnd. #27822

pull Brotcrunsher wants to merge 1 commits into bitcoin:master from Brotcrunsher:pushKVUB changing 13 files +26 −26
  1. Brotcrunsher commented at 4:35 PM on June 4, 2023: contributor

    Any identifier starting with 2 _ is reserved for the compiler and thus must not be used.

    See: https://stackoverflow.com/a/228797/7130273

  2. DrahtBot commented at 4:35 PM on June 4, 2023: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK MarcoFalke

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #27788 (rpc: Be able to access RPC parameters by name by achow101)
    • #27742 ([NO MERGE] BIP331 Ancestor Package Relay by glozow)

    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.

  3. hebasto commented at 5:08 PM on June 4, 2023: member
  4. Brotcrunsher force-pushed on Jun 4, 2023
  5. DrahtBot added the label CI failed on Jun 4, 2023
  6. DrahtBot removed the label CI failed on Jun 4, 2023
  7. maflcko commented at 8:35 AM on June 5, 2023: member

    lgtm ACK acad989e67a57709dbb882c97852c2067f9dc65e

  8. hebasto commented at 8:58 AM on June 5, 2023: member

    Does it make sense to force such changes with the Clang's -Wreserved-identifier diagnostic flag?

  9. DrahtBot added the label Needs rebase on Jun 9, 2023
  10. fanquake commented at 10:38 AM on June 15, 2023: member

    @Brotcrunsher are you going to rebase this?

  11. maflcko commented at 11:18 AM on June 15, 2023: member

    Does it make sense to force such changes with the Clang's -Wreserved-identifier diagnostic flag?

    I get a warning about _EraseTx and this:

    diff --git a/src/support/allocators/secure.h b/src/support/allocators/secure.h
    index a0918bf463..b2076bea07 100644
    --- a/src/support/allocators/secure.h
    +++ b/src/support/allocators/secure.h
    @@ -32,9 +32,9 @@ struct secure_allocator : public std::allocator<T> {
         {
         }
         ~secure_allocator() noexcept {}
    -    template <typename _Other>
    +    template <typename Other>
         struct rebind {
    -        typedef secure_allocator<_Other> other;
    +        typedef secure_allocator<Other> other;
         };
     
         T* allocate(std::size_t n, const void* hint = nullptr)
    diff --git a/src/support/allocators/zeroafterfree.h b/src/support/allocators/zeroafterfree.h
    index 795eea3bc0..2dc644c242 100644
    --- a/src/support/allocators/zeroafterfree.h
    +++ b/src/support/allocators/zeroafterfree.h
    @@ -27,9 +27,9 @@ struct zero_after_free_allocator : public std::allocator<T> {
         {
         }
         ~zero_after_free_allocator() noexcept {}
    -    template <typename _Other>
    +    template <typename Other>
         struct rebind {
    -        typedef zero_after_free_allocator<_Other> other;
    +        typedef zero_after_free_allocator<Other> other;
         };
     
         void deallocate(T* p, std::size_t n)
    
  12. hebasto commented at 11:32 AM on June 15, 2023: member

    Does it make sense to force such changes with the Clang's -Wreserved-identifier diagnostic flag?

    I get a warning about _EraseTx and this:

    I got the same. Could be added to this PR, no?

  13. Brotcrunsher force-pushed on Jun 20, 2023
  14. Brotcrunsher force-pushed on Jun 20, 2023
  15. Brotcrunsher force-pushed on Jun 20, 2023
  16. DrahtBot removed the label Needs rebase on Jun 20, 2023
  17. maflcko commented at 7:02 AM on June 20, 2023: member

    Please follow the commit subject max len, according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches

  18. scripted-diff: Following the C++ Standard rules for identifiers with _.
    Any identifier starting with two _, or one _ followed by a capital letter is reserved for the compiler and thus must not be used. See: https://stackoverflow.com/a/228797/7130273
    
    -BEGIN VERIFY SCRIPT-
    s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; }
    
    s '__pushKV' 'pushKVEnd'
    s '_EraseTx' 'EraseTxNoLock'
    s '_Other' 'Other'
    -END VERIFY SCRIPT-
    bdea2bb114
  19. Brotcrunsher force-pushed on Jun 20, 2023
  20. Brotcrunsher commented at 8:26 AM on June 20, 2023: contributor

    Please follow the commit subject max len

    Thanks for the feedback, done! One detail question regarding the max length (not important for this Commit, just want to learn for the future). Are CI triggers like "scripted-diff: " considered as part of the subject line and thus part of the 80 chars limitation?

  21. maflcko commented at 8:49 AM on June 20, 2023: member

    lgtm ACK bdea2bb1147bbd22f8b4fa406262470f9d084215

  22. fanquake commented at 10:00 AM on June 21, 2023: member

    Does it make sense to force such changes with the Clang's -Wreserved-identifier diagnostic flag?

    Doing so is going to require further suppressing, or changes in upstream code. i.e:

    ./minisketch/include/minisketch.h:2:9: warning: macro name is a reserved identifier [-Wreserved-macro-identifier]
    #define _MINISKETCH_H_ 1
            ^
    1 warning generated.
    ./crypto/ctaes/ctaes.h:8:9: warning: macro name is a reserved identifier [-Wreserved-macro-identifier]
    #define _CTAES_H_ 1
            ^
    1 warning generated.
    ./leveldb/port/thread_annotations.h:16:9: warning: macro name is a reserved identifier [-Wreserved-macro-identifier]
    #define THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x))
            ^
    1 warning generated.
    minisketch/src/util.h:16:11: warning: macro name is a reserved identifier [-Wreserved-macro-identifier]
    #  define __GNUC_PREREQ(_maj,_min) \
              ^
    3 warnings generated.
    
  23. fanquake merged this on Jun 21, 2023
  24. fanquake closed this on Jun 21, 2023

  25. sidhujag referenced this in commit 1fe6a622dd on Jun 21, 2023
  26. Frank-GER referenced this in commit 44a152da2d on Aug 28, 2023
  27. Fabcien referenced this in commit dafdf07d97 on Jun 6, 2024
  28. bitcoin locked this on Jun 20, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-26 06:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me