util: Add util::NotNull #34844

pull maflcko wants to merge 6 commits into bitcoin:master from maflcko:2603-not-null changing 8 files +541 −12
  1. maflcko commented at 7:42 pm on March 17, 2026: member

    The C++ standard library lacks a type to denote a smart pointer is not null. For raw pointer there is std::reference_wrapper, or a plain reference.

    Other third-party libraries provide such a type, such as gsl::strict_not_null. However, GSL’s not_null requires copies and disallows moves of the underlying smart pointer, which is expensive and makes is impossible to use on a std::unique_ptr in real code.

    Fix all issues by adding util::NotNull<SmartPtrType>, which documents (and checks) that the inner pointer is never null. Only when the inner pointer was moved, it becomes null, even inside NotNull. However, this is fine, because with clang-tidy use-after-free is forbidden, so it will never be possible to observe the silent inner nullptr.

    This type can be used when passing never-null smart pointers between functions. It removes the need to Assert() the pointer before dereference. For example, in a getter function:

     0util::NotNull<std::unique_ptr<Stats>> GetStats()
     1{
     2    return util::NotNull{std::make_unique<Stats>()};
     3}
     4
     5int main()
     6{
     7    auto stats{GetStats()};
     8    stats->foo; // This can never lead to a nullptr deref
     9    // Assert(stats)->foo; // This is redundant and won't compile
    10}
    

    Fixes https://github.com/bitcoin/bitcoin/issues/24423

  2. DrahtBot added the label Utils/log/libs on Mar 17, 2026
  3. DrahtBot commented at 7:43 pm on March 17, 2026: contributor

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK stickies-v

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

  4. maflcko force-pushed on Mar 17, 2026
  5. DrahtBot added the label CI failed on Mar 17, 2026
  6. DrahtBot commented at 8:03 pm on March 17, 2026: contributor

    🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/23213194537/job/67467180922 LLM reason (✨ experimental): Build failed during the cmake build, due to a compilation error in src/test/util_pointers_tests.cpp.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  7. stickies-v commented at 8:08 pm on March 17, 2026: contributor
    Concept ACK
  8. util: Add util::NotNull<AnyPtrType>
    Based on gsl::strict_not_null, with minimal modifications.
    
    It can be reviewed by calling
    
    curl -fL 'https://raw.githubusercontent.com/microsoft/GSL/756c91ab895aa52f650599bb1a3fc131f1f4b5ef/include/gsl/pointers' -o ./src/util/pointers.h
    
    and then confirming that the difference is documented and correct.
    fa0d7e6501
  9. util: Allow util::NotNull to be moved fa1dbbecb2
  10. util: Make util::NotNull swap noexcept
    This fixes the following clang-tidy warning:
    
    src/util/pointers.h:166:10: error: swap functions should be marked noexcept [performance-noexcept-swap,-warnings-as-errors]
      166 |     void swap(not_null<T>& other) { std::swap(ptr_, other.ptr_); }
    src/util/pointers.h:174:6: error: swap functions should be marked noexcept [performance-noexcept-swap,-warnings-as-errors]
      174 | void swap(not_null<T>& a, not_null<T>& b)
    fa2073887d
  11. refactor: Use util::NotNull<std::unique_ptr<LevelDBContext>> m_db_context
    This allows to drop the runtime Assert every time the context is
    accessed, because the type is known to be not null at compile-time.
    
    Note there is still the runtime Assert inside the util::NotNull
    constructor itself. However, this is called at most once, with the
    possibility of the compiler being able to optimize it away.
    fa4228fb45
  12. refactor: In CNode use util::NotNull<std::unique_ptr<Transport>> m_transport
    This documents (and checks) that the transport pointer is never null.
    fa1b301695
  13. refactor: Return util::NotNull<std::unique_ptr<ChangeSet>> from CTxMemPool::GetChangeSet()
    This documents (and checks) that the change set is never null.
    fa9851dc60
  14. maflcko force-pushed on Mar 17, 2026
  15. maflcko commented at 9:41 pm on March 17, 2026: member

    Looks like C++20 CTAD alias templates are not supported with clang-18, so I replaced this one-liner with a more verbose alternative in the last force push.

    0 template <class T> using NotNull = gsl_detail::strict_not_null<T>;
    
  16. DrahtBot removed the label CI failed on Mar 17, 2026

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-03-23 06:13 UTC

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