Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warnings in build process #1470

Merged
merged 4 commits into from
Aug 17, 2024
Merged

Fix warnings in build process #1470

merged 4 commits into from
Aug 17, 2024

Conversation

psolstice
Copy link
Contributor

@psolstice psolstice commented Jul 15, 2024

Supersedes #1423

Update lib immer to a more recent version (v0.8.1 5875f7739a6c642ad58cbedadb509c86d4212). This fixes a lot of random warnings we have in lib immer.
Fixed warnings in Linux / Clang.
Fixed warnings in Win cross-compile.

aleflm and others added 3 commits July 15, 2024 07:49
Copy link

coderabbitai bot commented Jul 15, 2024

Walkthrough

The changes encompass extensive updates to the immer library, including reorganization and addition of headers, enhancements to algorithm functionalities, and improved exception handling. Key modifications include new utilities for data structures, adjustments to macros and configurations, and updates to array and node components. These changes improve readability, maintainability, and the overall robustness of the library.

Changes

Files/Paths Change Summaries
src/Makefile.am Reorganized and updated immer headers, changes in immer directory structure.
src/compat/glibcxx_sanity.cpp Suppressed unused return value warning in sanity_test_range_fmt.
src/immer/algorithm.hpp Added includes, refactored accumulate function, introduced differ and diff functionalities.
src/immer/{array.hpp,array_transient.hpp} Added IMMER_NODISCARD, new member functions, and reordered includes.
src/immer/atom.hpp Renamed types and variables, adjustments to move constructors and method signatures.
src/immer/box.hpp Added #include <cstddef>, reformatted constructors, added comparison operators.
src/immer/config.hpp Introduced macros for C++17 features, exception handling, debugging options, and optimizations.
src/immer/detail/arrays/{no_capacity.hpp, node.hpp} Added includes, refactored methods, improved exception handling, and assertions.
src/immer/detail/arrays/with_capacity.hpp Added includes, refactored constructor and destructor, added data_mut method.
src/immer/detail/combine_standard_layout.hpp Added includes, refactored struct definitions, improved formatting and conditional logic.
src/immer/detail/hamts/{bits.hpp, champ_iterator.hpp} Added includes, refactored functions and type definitions, added assertions, modified end_ calculation.

Poem

Amidst the code, where changes brew,
New headers dawn, old ones bid adieu.
Arrays and atoms, they transform with grace,
Refactored lines, in a seamless embrace.
🎉 To algorithms that now differ and shine,
Here's to progress, in every define! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)
src/immer/experimental/dvektor.hpp (1)

Missing Implementations and Tests for dvektor Member Functions

The member functions push_back, assoc, and update in the dvektor class are not found in the codebase. Additionally, there are no corresponding test cases for these functions.

  • Ensure that the member functions push_back, assoc, and update are implemented in the dvektor class.
  • Add test cases for these functions to verify their functionality.
Analysis chain

Line range hint 25-67:
Changes to dvektor class improve functionality.

The added type aliases, iterators, and member functions enhance the functionality and usability of the dvektor class.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation and tests of the new member functions in `dvektor` class.

# Test: Search for the implementation of the new member functions. Expect: Correct implementation in the included headers.
rg --type cpp --pattern $'dvektor::push_back'
rg --type cpp --pattern $'dvektor::assoc'
rg --type cpp --pattern $'dvektor::update'

# Test: Search for the tests of the new member functions. Expect: Test cases in the test files.
rg --type cpp --pattern $'dvektor::push_back' --files-with-matches | xargs -I {} rg --type cpp --pattern $'TEST' {}
rg --type cpp --pattern $'dvektor::assoc' --files-with-matches | xargs -I {} rg --type cpp --pattern $'TEST' {}
rg --type cpp --pattern $'dvektor::update' --files-with-matches | xargs -I {} rg --type cpp --pattern $'TEST' {}

Length of output: 2940


Script:

#!/bin/bash
# Description: Verify the implementation and tests of the new member functions in `dvektor` class.

# Test: Search for the implementation of the new member functions. Expect: Correct implementation in the included headers.
rg --type cpp 'dvektor::push_back'
rg --type cpp 'dvektor::assoc'
rg --type cpp 'dvektor::update'

# Test: Search for the tests of the new member functions. Expect: Test cases in the test files.
rg --type cpp 'dvektor::push_back' --files-with-matches | xargs -I {} rg --type cpp 'TEST' {}
rg --type cpp 'dvektor::assoc' --files-with-matches | xargs -I {} rg --type cpp 'TEST' {}
rg --type cpp 'dvektor::update' --files-with-matches | xargs -I {} rg --type cpp 'TEST' {}

Length of output: 378


Script:

#!/bin/bash
# Description: Verify the implementation and tests of the new member functions in `dvektor` class.

# Search for the `dvektor` class definition to ensure we are looking at the right files.
rg --type cpp 'class dvektor'

# Search for the implementation of the new member functions in the `dvektor` class.
rg --type cpp 'dvektor::push_back'
rg --type cpp 'dvektor::assoc'
rg --type cpp 'dvektor::update'

# Search for the tests of the new member functions in the `dvektor` class.
rg --type cpp 'dvektor::push_back' --files-with-matches | xargs -I {} rg --type cpp 'TEST' {}
rg --type cpp 'dvektor::assoc' --files-with-matches | xargs -I {} rg --type cpp 'TEST' {}
rg --type cpp 'dvektor::update' --files-with-matches | xargs -I {} rg --type cpp 'TEST' {}

Length of output: 459


Script:

#!/bin/bash
# Description: Verify the implementation and tests of the new member functions in `dvektor` class.

# Search for the `dvektor` class definition to ensure we are looking at the right files.
rg --type cpp 'class dvektor'

# Search for the implementation of the new member functions in the `dvektor` class.
rg --type cpp 'dvektor::push_back'
rg --type cpp 'dvektor::assoc'
rg --type cpp 'dvektor::update'

# Search for the tests of the new member functions in the `dvektor` class.
rg --type cpp 'push_back' --files-with-matches | xargs -I {} rg --type cpp 'TEST' {}
rg --type cpp 'assoc' --files-with-matches | xargs -I {} rg --type cpp 'TEST' {}
rg --type cpp 'update' --files-with-matches | xargs -I {} rg --type cpp 'TEST' {}

Length of output: 56225

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between bcaacd4 and 9391c46.

Files selected for processing (56)
  • src/Makefile.am (1 hunks)
  • src/compat/glibcxx_sanity.cpp (1 hunks)
  • src/immer/algorithm.hpp (11 hunks)
  • src/immer/array.hpp (9 hunks)
  • src/immer/array_transient.hpp (6 hunks)
  • src/immer/atom.hpp (8 hunks)
  • src/immer/box.hpp (6 hunks)
  • src/immer/config.hpp (3 hunks)
  • src/immer/detail/arrays/no_capacity.hpp (4 hunks)
  • src/immer/detail/arrays/node.hpp (3 hunks)
  • src/immer/detail/arrays/with_capacity.hpp (10 hunks)
  • src/immer/detail/combine_standard_layout.hpp (8 hunks)
  • src/immer/detail/hamts/bits.hpp (5 hunks)
  • src/immer/detail/hamts/champ.hpp (10 hunks)
  • src/immer/detail/hamts/champ_iterator.hpp (6 hunks)
  • src/immer/detail/hamts/node.hpp (13 hunks)
  • src/immer/detail/iterator_facade.hpp (5 hunks)
  • src/immer/detail/rbts/bits.hpp (2 hunks)
  • src/immer/detail/rbts/node.hpp (23 hunks)
  • src/immer/detail/rbts/position.hpp (50 hunks)
  • src/immer/detail/rbts/rbtree.hpp (16 hunks)
  • src/immer/detail/rbts/rbtree_iterator.hpp (4 hunks)
  • src/immer/detail/rbts/rrbtree.hpp (27 hunks)
  • src/immer/detail/rbts/rrbtree_iterator.hpp (3 hunks)
  • src/immer/detail/rbts/visitor.hpp (1 hunks)
  • src/immer/detail/ref_count_base.hpp (1 hunks)
  • src/immer/detail/type_traits.hpp (2 hunks)
  • src/immer/detail/util.hpp (5 hunks)
  • src/immer/experimental/detail/dvektor_impl.hpp (18 hunks)
  • src/immer/experimental/dvektor.hpp (2 hunks)
  • src/immer/flex_vector.hpp (15 hunks)
  • src/immer/flex_vector_transient.hpp (7 hunks)
  • src/immer/heap/cpp_heap.hpp (2 hunks)
  • src/immer/heap/debug_size_heap.hpp (2 hunks)
  • src/immer/heap/free_list_heap.hpp (1 hunks)
  • src/immer/heap/free_list_node.hpp (1 hunks)
  • src/immer/heap/gc_heap.hpp (4 hunks)
  • src/immer/heap/heap_policy.hpp (6 hunks)
  • src/immer/heap/malloc_heap.hpp (3 hunks)
  • src/immer/heap/split_heap.hpp (2 hunks)
  • src/immer/heap/tags.hpp (1 hunks)
  • src/immer/heap/thread_local_free_list_heap.hpp (2 hunks)
  • src/immer/heap/unsafe_free_list_heap.hpp (5 hunks)
  • src/immer/lock/no_lock_policy.hpp (1 hunks)
  • src/immer/lock/spinlock_policy.hpp (1 hunks)
  • src/immer/map.hpp (8 hunks)
  • src/immer/map_transient.hpp (1 hunks)
  • src/immer/memory_policy.hpp (8 hunks)
  • src/immer/refcount/no_refcount_policy.hpp (1 hunks)
  • src/immer/refcount/refcount_policy.hpp (1 hunks)
  • src/immer/refcount/unsafe_refcount_policy.hpp (1 hunks)
  • src/immer/set.hpp (3 hunks)
  • src/immer/set_transient.hpp (1 hunks)
  • src/immer/table.hpp (1 hunks)
  • src/immer/table_transient.hpp (1 hunks)
  • src/immer/transience/gc_transience_policy.hpp (5 hunks)
Files not processed due to max files limit (13)
  • src/immer/transience/no_transience_policy.hpp
  • src/immer/vector.hpp
  • src/immer/vector_transient.hpp
  • src/libspark/bpplus.cpp
  • src/rpc/net.cpp
  • src/rpc/protocol.cpp
  • src/script/ismine.cpp
  • src/secp256k1/src/ecmult.h
  • src/secp256k1/src/ecmult_const_impl.h
  • src/secp256k1/src/ecmult_impl.h
  • src/secp256k1/src/scratch_impl.h
  • src/util.cpp
  • src/wallet/wallet.cpp
Files not summarized due to errors (4)
  • src/immer/detail/hamts/champ.hpp: Error: Message exceeds token limit
  • src/immer/detail/hamts/node.hpp: Error: Message exceeds token limit
  • src/immer/detail/rbts/position.hpp: Error: Message exceeds token limit
  • src/immer/detail/rbts/rrbtree.hpp: Error: Message exceeds token limit
Files skipped from review due to trivial changes (12)
  • src/compat/glibcxx_sanity.cpp
  • src/immer/detail/rbts/bits.hpp
  • src/immer/detail/rbts/rrbtree_iterator.hpp
  • src/immer/detail/ref_count_base.hpp
  • src/immer/detail/type_traits.hpp
  • src/immer/heap/free_list_heap.hpp
  • src/immer/heap/free_list_node.hpp
  • src/immer/heap/heap_policy.hpp
  • src/immer/heap/split_heap.hpp
  • src/immer/heap/tags.hpp
  • src/immer/heap/thread_local_free_list_heap.hpp
  • src/immer/refcount/no_refcount_policy.hpp
Additional comments not posted (241)
src/immer/lock/no_lock_policy.hpp (1)

1-25: LGTM!

The changes in this file are straightforward and don't introduce any issues.

src/immer/refcount/unsafe_refcount_policy.hpp (1)

26-30: LGTM!

The changes in this file are straightforward and don't introduce any issues.

src/immer/refcount/refcount_policy.hpp (1)

27-31: LGTM!

The changes in this file are straightforward and don't introduce any issues.

src/immer/heap/cpp_heap.hpp (1)

36-38: LGTM!

The changes in this file are straightforward and don't introduce any issues.

src/immer/heap/malloc_heap.hpp (4)

13-13: Include <cassert> is appropriate.

The addition of <cassert> is necessary for using assertions.


15-15: Include <exception> is appropriate.

The addition of <exception> is necessary for using standard exception classes like std::bad_alloc.


16-16: Include <memory> is appropriate.

The addition of <memory> is necessary for using memory management utilities.


34-34: Improved performance and exception handling in allocate function.

The use of IMMER_UNLIKELY and IMMER_THROW enhances performance and exception handling.

Verification successful

Improved performance and exception handling in allocate function.

The use of IMMER_UNLIKELY and IMMER_THROW enhances performance and exception handling. The macros are correctly defined in src/immer/config.hpp.

  • IMMER_UNLIKELY(cond) is defined as __builtin_expect(!!(cond), 0).
  • IMMER_THROW(expr) is defined as throw expr.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition of IMMER_UNLIKELY and IMMER_THROW macros.

# Test: Search for the definition of the macros. Expect: Definitions in the included headers.
rg --type cpp --pattern $'#define IMMER_UNLIKELY'
rg --type cpp --pattern $'#define IMMER_THROW'

Length of output: 922


Script:

#!/bin/bash
# Description: Verify the definition of IMMER_UNLIKELY and IMMER_THROW macros.

# Test: Search for the definition of the macros. Expect: Definitions in the included headers.
rg '#define IMMER_UNLIKELY'
rg '#define IMMER_THROW'

Length of output: 351

src/immer/detail/rbts/visitor.hpp (1)

24-24: Use of IMMER_UNREACHABLE in visit_node function is appropriate.

The use of IMMER_UNREACHABLE is suitable for indicating unreachable code.

Verification successful

Use of IMMER_UNREACHABLE in visit_node function is appropriate.

The IMMER_UNREACHABLE macro is correctly defined in src/immer/config.hpp and is suitable for indicating unreachable code.

  • src/immer/config.hpp: #define IMMER_UNREACHABLE __assume(false)
  • src/immer/config.hpp: #define IMMER_UNREACHABLE __builtin_unreachable()
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition of IMMER_UNREACHABLE macro.

# Test: Search for the definition of the macro. Expect: Definition in the included headers.
rg --type cpp --pattern $'#define IMMER_UNREACHABLE'

Length of output: 465


Script:

#!/bin/bash
# Description: Verify the definition of IMMER_UNREACHABLE macro.

# Test: Search for the definition of the macro. Expect: Definition in the included headers.
rg '#define IMMER_UNREACHABLE'

Length of output: 166

src/immer/lock/spinlock_policy.hpp (5)

1-8: License header is appropriate.

The addition of the license header is necessary for legal compliance.


11-11: Include <atomic> is appropriate.

The addition of <atomic> is necessary for using atomic operations.


12-12: Include <thread> is appropriate.

The addition of <thread> is necessary for using thread operations.


15-21: Macro definitions for IMMER_SMT_PAUSE are appropriate.

The addition of these macros is necessary for handling different compiler and architecture-specific pause instructions.


28-66: Implementation of spinlock_policy is appropriate.

The spinlock_policy struct provides a lightweight locking mechanism using atomic operations.

src/immer/experimental/dvektor.hpp (1)

15-15: Include <cstddef> is appropriate.

The addition of <cstddef> is necessary for using size-related types.

src/immer/heap/debug_size_heap.hpp (4)

14-17: LGTM! Necessary headers added.

The added headers <cassert>, <cstddef>, <type_traits>, and <memory> are appropriate and necessary for the functionality provided in this file.


30-43: Conditional Compilation for MinGW 32-bit: Approved

The conditional compilation block addresses a known bug in MinGW 32-bit regarding std::max_align_t. The logic is sound and correctly implemented.


48-50: LGTM! Allocation logic is correct.

The allocate function correctly includes the extra_size in the allocation size and stores the original size at the beginning of the allocated memory.


Line range hint 52-55:
LGTM! Deallocation logic is correct.

The deallocate function correctly retrieves the original size from the allocated memory and asserts that it matches the size passed to the function.

src/immer/detail/rbts/rbtree_iterator.hpp (6)

12-12: LGTM! Necessary header added.

The added header <immer/detail/rbts/rbtree.hpp> is appropriate and necessary for the functionality provided in this file.


29-30: LGTM! Addition of end_t struct is correct.

The addition of end_t struct within rbtree_iterator is logical and correctly implemented.


35-45: LGTM! Constructor changes are correct.

The constructors of rbtree_iterator correctly initialize the member variables.


54-56: LGTM! Member variable changes are correct.

The changes in the member variables of rbtree_iterator are logical and correctly implemented.


78-78: LGTM! equal function is correct.

The equal function correctly compares two rbtree_iterator instances.


82-83: LGTM! distance_to function is correct.

The distance_to function correctly calculates the distance between two rbtree_iterator instances.

src/immer/transience/gc_transience_policy.hpp (5)

13-13: LGTM! Necessary headers added.

The added headers <atomic>, <memory>, and <utility> are appropriate and necessary for the functionality provided in this file.


43-45: LGTM! Changes in edit struct are correct.

The changes in the edit struct, including the constructor and comparison operators, are logical and correctly implemented.


Line range hint 60-66:
LGTM! Changes in owner struct are correct.

The changes in the owner struct, including the make_token_ function and modifications in constructors and assignment operators, are logical and correctly implemented.


88-88: LGTM! Changes in ownee struct are correct.

The changes in the ownee struct, including the token_ member variable and modifications in methods, are logical and correctly implemented.


111-111: LGTM! Addition of noone static member variable is correct.

The addition of noone static member variable to the gc_transience_policy struct is logical and correctly implemented.

src/immer/heap/unsafe_free_list_heap.hpp (4)

13-15: LGTM! Necessary headers added.

The added headers <cassert> and <cstddef> are appropriate and necessary for the functionality provided in this file.


31-31: LGTM! Initialization of static head_t is correct.

The initialization of static head_t within the head function is logical and correctly implemented.


72-73: LGTM! Deallocation logic is correct.

The deallocate function correctly handles the free list and base deallocation.


83-84: LGTM! Clear function logic is correct.

The clear function correctly deallocates all nodes in the free list.

src/immer/heap/gc_heap.hpp (4)

20-22: Includes for <cassert>, <cstdlib>, and <exception> are necessary.

These includes are required for assertions, memory allocation, and exception handling.


Line range hint 27-63:
Garbage collection initialization macros and code are necessary.

These changes ensure that garbage collection is initialized if required.


Line range hint 110-119:
Exception handling for memory allocation failures is necessary.

These changes ensure that exceptions are properly thrown if memory allocation fails.


123-126: Deallocate method is necessary.

This method ensures that memory is properly deallocated.

src/immer/config.hpp (7)

11-13: C++17 detection macro is necessary.

This change ensures that the code can conditionally use C++17 features if available.


15-23: nodiscard attribute detection macro is necessary.

This change ensures that functions marked with nodiscard will generate a warning if the return value is ignored.


25-29: Exception handling detection macro is necessary.

This change ensures that the code can conditionally handle the absence of exceptions.


31-45: Exception handling macros are necessary.

These changes ensure that exception handling can be conditionally compiled based on the presence of exceptions.


47-49: nodiscard macro definition is necessary.

This change ensures that the nodiscard attribute is always available.


51-63: Tagged node debugging macros are necessary.

These changes ensure that tagged node debugging can be conditionally enabled or disabled.


Line range hint 64-122:
Debugging and optimization macros are necessary.

These changes ensure that debugging and optimization features can be conditionally enabled or disabled.

src/immer/detail/arrays/node.hpp (7)

11-16: Includes for <immer/config.hpp> and <cstddef> are necessary.

These includes are required for configuration settings and size-related operations.


26-59: Updates to the node structure are necessary.

These changes improve the functionality and readability of the node structure.


64-64: Updates to the delete_n method are necessary.

These changes ensure that memory is properly destroyed and deallocated.


69-75: Updates to the make_n and make_e methods are necessary.

These changes ensure that memory is properly allocated.


83-89: Updates to the fill_n method are necessary.

These changes ensure that memory is properly initialized and exceptions are handled.


93-106: Updates to the copy_n method are necessary.

These changes ensure that memory is properly copied and exceptions are handled.


Line range hint 118-122:
Updates to the copy_e method are necessary.

These changes ensure that memory is properly copied and exceptions are handled.

src/immer/detail/hamts/champ_iterator.hpp (5)

Line range hint 32-59:
Updates to the champ_iterator structure are necessary.

These changes improve the functionality and readability of the champ_iterator structure.


72-72: Update to the increment method is necessary.

This change ensures that the iterator remains valid after incrementing.


84-95: Updates to the step_down method are necessary.

These changes ensure that the iterator can step down correctly.


109-118: Updates to the step_right method are necessary.

These changes ensure that the iterator can step right correctly.


Line range hint 131-146:
Updates to the ensure_valid_ method are necessary.

These changes ensure that the iterator remains valid.

src/immer/detail/hamts/bits.hpp (7)

11-11: Include directive for <cstddef> is necessary.

The inclusion of <cstddef> is required for the use of std::size_t.


22-26: Type alias definitions improve readability and maintainability.

The type aliases size_t, hash_t, bits_t, count_t, and shift_t provide meaningful names for commonly used types.


Line range hint 31-51:
Template struct get_bitmap_type and its specializations are well-implemented.

The template struct and its specializations provide a mechanism to select an appropriate bitmap type based on the value of B.


53-63: Template constants provide compile-time calculations.

The branches, mask, max_depth, and max_shift template constants enable compile-time calculations for various parameters based on the value of B.


82-83: Fallback implementation for popcount on std::uint64_t.

The popcount_fallback function provides a fallback implementation for counting the number of set bits in a 64-bit integer.


102-112: Platform-specific implementation for popcount on std::uint64_t.

The popcount function provides a platform-specific implementation for counting the number of set bits in a 64-bit integer, with a fallback to the popcount_fallback function.


128-171: Class template set_bits_range provides an iterator-based interface.

The set_bits_range class template introduces an iterator-based interface for iterating over set bits in a bitmap, enhancing code readability and usability.

src/immer/memory_policy.hpp (7)

13-14: Include directives for no_lock_policy.hpp and spinlock_policy.hpp are necessary.

The inclusion of no_lock_policy.hpp and spinlock_policy.hpp is required for the use of no_lock_policy and spinlock_policy in the file.


30-32: Metafunction get_transience_policy selects the appropriate transience policy.

The get_transience_policy metafunction selects the appropriate transience policy based on the provided refcount policy.


44-46: Metafunction get_prefer_fewer_bigger_objects determines preference for fewer bigger objects.

The get_prefer_fewer_bigger_objects metafunction determines whether to prefer fewer bigger objects based on the provided heap policy.


59-61: Metafunction get_use_transient_rvalues determines whether to use transient R-values.

The get_use_transient_rvalues metafunction determines whether to use transient R-values based on the provided refcount policy.


88-104: Struct memory_policy encapsulates various memory management policies and flags.

The memory_policy struct encapsulates various memory management policies and flags, providing a flexible and configurable memory policy.


129-132: Default lock policy is set based on thread safety configuration.

The default lock policy is set to no_lock_policy if thread safety is disabled, otherwise to spinlock_policy.


138-140: Default memory policy is configured with appropriate default policies.

The default memory policy is set using the memory_policy struct with default policies for heap, refcount, and lock.

src/immer/detail/combine_standard_layout.hpp (7)

11-12: Includes added for necessary utilities.

The includes for <cstddef> and <type_traits> are necessary for std::size_t and type traits utilities used in the file.


14-19: Macros added for GCC 7.1 standard layout detection issue.

The macros handle a known issue with standard layout detection in GCC 7.1 by providing a workaround.


52-53: Type wrapper struct added.

The type_t struct template is a simple wrapper for a type, used for type-based dispatch in the get functions.


61-79: Struct template for inheriting types added and modified.

The inherit struct template combines standard layout types while preserving empty base optimizations and supports type-based dispatch.


Line range hint 107-125:
Struct template for member types added and modified.

The member struct template combines standard layout types by adding them as members and supports type-based dispatch.


164-186: Struct template for combining two types added.

The member_two struct template combines two standard layout types, one as a member and the other as a nested member, and supports type-based dispatch.


Line range hint 198-219:
Struct template for combining multiple types added and modified.

The combine_standard_layout_aux struct template combines multiple standard layout types while preserving empty base optimizations and standard layout rules.

src/immer/array_transient.hpp (3)

12-14: Includes added for necessary utilities.

The includes for <cstddef> and <immer/memory_policy.hpp> are necessary for std::size_t and memory policy utilities used in the file.


64-96: Attributes added to prevent discarding return values.

The IMMER_NODISCARD attribute ensures that the return value of these functions is not ignored, which is important for correctness.

Also applies to: 102-107, 117-122, 130-138, 184-191


109-112: Member function added for mutable data access.

The data_mut() member function provides mutable access to the underlying data, which is necessary for modifying it in a controlled manner.

src/immer/set_transient.hpp (3)

12-12: Includes added for necessary utilities.

The include for <immer/memory_policy.hpp> is necessary for memory policy utilities used in the file.


63-84: Attributes added to prevent discarding return values.

The IMMER_NODISCARD attribute ensures that the return value of these functions is not ignored, which is important for correctness.

Also applies to: 94-102, 108-112, 120-124, 135-142, 162-167


149-150: Member functions modified for better readability and consistency.

The modifications improve the readability and consistency of the code, making it easier to understand and maintain.

Also applies to: 156-157, 164-167

src/immer/box.hpp (3)

14-15: Includes added for necessary utilities.

The include for <cstddef> is necessary for std::size_t used in the file.


125-125: Attributes added to prevent discarding return values.

The IMMER_NODISCARD attribute ensures that the return value of these functions is not ignored, which is important for correctness.

Also applies to: 131-134, 152-157, 167-225


167-225: Comparison operators for box instances added.

The comparison operators allow for comparing box instances, which is useful for various operations and improves usability.

src/immer/atom.hpp (6)

24-28: LGTM!

The renaming and reorganization of types and variables within refcount_atom_impl improve readability and consistency.


57-57: LGTM!

The use of scoped_lock_t in the exchange method ensures thread safety.


70-70: LGTM!

The use of scoped_lock_t in the update method ensures thread safety.


84-90: LGTM!

The renaming and reorganization of types and variables within gc_atom_impl improve readability and consistency. The use of static_assert ensures correct usage.


105-105: LGTM!

The changes in the exchange method ensure consistency with the rest of the class.


148-148: LGTM!

The addition of the IMMER_NODISCARD attribute ensures that the return value of the atom class functions is not ignored.

src/immer/flex_vector_transient.hpp (7)

91-91: LGTM!

The addition of the IMMER_NODISCARD attribute ensures that the return value of the begin function is not ignored.


97-100: LGTM!

The addition of the IMMER_NODISCARD attribute ensures that the return value of the end function is not ignored.


107-110: LGTM!

The addition of the IMMER_NODISCARD attribute ensures that the return value of the rbegin function is not ignored.


117-120: LGTM!

The addition of the IMMER_NODISCARD attribute ensures that the return value of the rend function is not ignored.


126-126: LGTM!

The addition of the IMMER_NODISCARD attribute ensures that the return value of the size function is not ignored.


132-132: LGTM!

The addition of the IMMER_NODISCARD attribute ensures that the return value of the empty function is not ignored.


231-236: LGTM!

The addition of the IMMER_NODISCARD attribute ensures that the return value of the persistent function is not ignored.

src/immer/detail/arrays/with_capacity.hpp (6)

104-112: LGTM!

The addition of the data_mut method provides a mutable pointer to the data, ensuring thread safety with edit_t.


124-135: LGTM!

The addition of the from_range method provides a way to create a with_capacity object from a range of iterators. The use of std::enable_if_t ensures correct usage.


147-147: LGTM!

The addition of the from_fill method provides a way to create a with_capacity object by filling it with a specified value.


167-167: LGTM!

The use of the IMMER_THROW macro ensures consistent exception handling. The addition of bounds checking improves robustness.


201-205: LGTM!

The use of the IMMER_TRY, IMMER_CATCH, and IMMER_RETHROW macros ensures consistent exception handling, improving robustness.


262-269: LGTM!

The use of the IMMER_TRY, IMMER_CATCH, and IMMER_RETHROW macros ensures consistent exception handling, improving robustness.

src/immer/detail/util.hpp (6)

27-38: LGTM!

The addition of the as_const method provides a way to obtain a const reference or pointer to a non-const object, ensuring const-correctness.


44-52: LGTM!

The addition of the auto_const_cast method provides a way to cast away constness from an object, useful in certain scenarios.


54-66: LGTM!

The addition of the destroy_at method provides a way to destroy an object at a given memory location, useful for custom memory management.


119-132: LGTM!

The use of the IMMER_TRY, IMMER_CATCH, and IMMER_RETHROW macros ensures consistent exception handling, improving robustness.


145-157: LGTM!

The use of the IMMER_TRY, IMMER_CATCH, and IMMER_RETHROW macros ensures consistent exception handling, improving robustness.


209-218: LGTM!

The addition of the log2 method provides a way to compute the base-2 logarithm of a number. The use of clz_ ensures platform optimization.

src/immer/algorithm.hpp (7)

12-13: Includes look good.

The new includes <cassert> and <functional> are appropriate for the added functionality.


52-53: LGTM!

The assertion and function call ensure that the iterators belong to the same container and the function object is forwarded correctly.


86-87: LGTM!

The assertion and function call ensure that the iterators belong to the same container and the function object is forwarded correctly.


96-112: Efficient use of move semantics.

The accumulate_move functions use std::move to efficiently accumulate values, which is beneficial for types that support move semantics.


122-132: Efficient accumulation using move semantics.

The accumulate function templates now use detail::accumulate_move, ensuring efficient accumulation using move semantics.

Also applies to: 144-154


232-277: Flexible difference handling.

The differ struct template and related functions provide a flexible way to handle differences between containers.

Also applies to: 306-321


306-321: Efficient difference computation.

The diff function template leverages structural sharing to offer efficient complexity for detecting changes.

src/immer/set.hpp (2)

13-13: Include looks good.

The new include <immer/memory_policy.hpp> is appropriate for the added functionality.


65-67: Improvements to set class template.

The changes include new type definitions, member functions, and improvements to existing functions. The additions and modifications are well-implemented and adhere to best practices.

Also applies to: 74-85, 96-113, 120-142, 151-158, 165-169, 177-181, 192-199, 204-211, 218-225, 232-239, 245-252, 260-260, 268-276, 278-278

src/immer/table_transient.hpp (2)

3-4: Includes look good.

The new includes <immer/detail/hamts/champ.hpp> and <immer/memory_policy.hpp> are appropriate for the added functionality.


35-38: Improvements to table_transient class template.

The changes include new type definitions, member functions, and improvements to existing functions. The additions and modifications are well-implemented and adhere to best practices.

Also applies to: 40-53, 58-59, 65-87, 96-114, 125-132, 140-144, 155-162, 170-174, 182-186, 197-204, 213-214, 220-227, 235-242, 248-248, 254-258, 264-264, 267-279

src/immer/array.hpp (2)

12-13: Include looks good.

The new include <immer/memory_policy.hpp> is appropriate for the added functionality.


45-48: Improvements to array class template.

The changes include new type definitions, member functions, and improvements to existing functions. The additions and modifications are well-implemented and adhere to best practices.

Also applies to: 54-133, 139-171, 179-191, 208-216, 234-242, 262-271, 289-297, 303-310, 318-318, 326-370

src/immer/map_transient.hpp (2)

12-12: Include directive for memory_policy.hpp added.

This inclusion is necessary for the MemoryPolicy template parameter used in the class.


18-316: Class template map_transient defined with new template parameters and methods.

The class template definition and methods align with the overall structure of the immer library.

src/immer/experimental/detail/dvektor_impl.hpp (2)

20-20: Include directive for <cstddef> added.

This inclusion is necessary for the use of std::size_t and other size-related types.


Line range hint 39-531: Class template node and related methods defined or refactored.

The class template definition and methods align with the overall structure of the immer library.

src/immer/detail/rbts/rbtree.hpp (2)

20-20: Include directive for <stdexcept> added.

This inclusion is necessary for the use of standard exception handling classes.


Line range hint 26-531: Class template rbtree and related methods defined or refactored.

The class template definition and methods align with the overall structure of the immer library.

src/immer/map.hpp (2)

18-18: Include directive for <stdexcept> added.

This inclusion is necessary for the use of standard exception handling classes.


Line range hint 44-542: Class template map and related methods defined or refactored.

The class template definition and methods align with the overall structure of the immer library.

src/immer/table.hpp (2)

1-7: LGTM! Necessary includes added.

The added include directives are appropriate for the functionality of the table class and related structures.


241-243: LGTM! Use of IMMER_NODISCARD is appropriate.

The IMMER_NODISCARD attribute ensures that the return values of these functions are not ignored, which is a good practice.

Also applies to: 247-249, 256-257, 262-263, 275-279, 286-290, 301-307, 315-318, 329-335, 343-346, 354-358, 372-374, 378-379, 395-397, 407-410, 419-423, 426-429, 438-440, 444-447, 455-456, 462-465, 471-474, 480-483

src/immer/flex_vector.hpp (1)

90-90: LGTM! Use of IMMER_NODISCARD is appropriate.

The IMMER_NODISCARD attribute ensures that the return values of these functions are not ignored, which is a good practice.

Also applies to: 142-144, 148-151, 158-161, 168-171, 177-179, 183-185, 188-190, 193-195, 201-204, 212-214, 217-220, 222-224, 241-244, 246-249, 266-269, 288-291, 293-296, 318-321, 324-327, 345-348, 350-353, 371-374, 376-379, 397-401, 403-407, 409-413, 415-419, 437-440, 441-446, 449-452, 453-457, 462-465, 471-474, 503-504, 506-515, 541-544, 546-548, 551-554, 556-558, 562-565, 568-570, 573-576, 578-580, 583-586, 588-590, 593-597, 599-603, 605-609, 611-614

src/Makefile.am (1)

270-340: LGTM! Necessary headers added.

The added headers are appropriate for the functionality of the lib immer library.

src/immer/detail/rbts/node.hpp (9)

11-15: Approved: Inclusion of new headers.

The added headers <immer/config.hpp>, <immer/detail/combine_standard_layout.hpp>, <immer/detail/rbts/bits.hpp>, <immer/detail/util.hpp>, and <immer/heap/tags.hpp> are relevant and necessary for the new functionality and refactored code.


Line range hint 26-52:
Approved: Modifications to node struct template.

The changes to the node struct template, including the addition of new member types and constants, and the introduction of relaxed_data_t struct, are relevant and necessary for the new functionality and optimizations.


56-58: Approved: Introduction of new type aliases.

The new type aliases relaxed_data_with_meta_t and relaxed_data_no_meta_t using combine_standard_layout_t are relevant and necessary for supporting different memory layouts based on the embed_relaxed constant.


71-78: Approved: Modifications to inner_t struct and data_t union.

The changes to the inner_t struct, including the addition of a pointer to relaxed_t, and the modifications to the data_t union are relevant and necessary for new functionality and optimizations.


83-89: Approved: Conditional inclusion of kind_t in impl_data_t struct.

The modification to conditionally include kind_t in the impl_data_t struct based on the IMMER_TAGGED_NODE macro is relevant and necessary for supporting conditional compilation for tagged nodes.


Line range hint 100-151:
Approved: Introduction of new constexpr functions and constants.

The new constexpr functions (sizeof_packed_leaf_n, sizeof_packed_inner_n, sizeof_packed_relaxed_n, sizeof_packed_inner_r_n) and the max_sizeof_* constants are relevant and necessary for new functionality and optimizations. The modifications to the sizeof_*_n functions to use keep_headroom are also appropriate.


163-187: Approved: Introduction of new member functions.

The new member functions relaxed, inner, and leaf are relevant and necessary for new functionality and optimizations. The inclusion of assertions ensures correctness.


202-218: Approved: Introduction of new static member functions.

The new static member functions make_inner_n_into, make_inner_n, and make_inner_e are relevant and necessary for creating and initializing inner nodes.


Line range hint 222-252:
Approved: Introduction of new static member functions.

The new static member functions make_inner_r_n, make_inner_sr_n, and make_inner_r_e are relevant and necessary for creating and initializing relaxed inner nodes.

src/immer/detail/hamts/node.hpp (16)

11-17: Include necessary headers for C++ standard library.

The inclusion of <immer/config.hpp>, <immer/detail/combine_standard_layout.hpp>, <immer/detail/hamts/bits.hpp>, <immer/detail/util.hpp>, <cassert>, and <cstddef> is appropriate and necessary for the functionality provided in this file.


23-26: Ensure compatibility with C++14 and MSVC.

The destroy_at function template is correctly defined to support C++14 and handle MSVC corner cases.


Line range hint 35-64:
Correct use of type aliases and nested structs.

The node struct definition and associated type aliases, such as node_t, memory, heap_policy, heap, transience, refs_t, ownee_t, edit_t, value_t, bitmap_t, and nested structs collision_t and values_data_t are correctly defined and necessary for the functionality provided in this file.


64-69: Correct use of values_t type alias and inner_t struct.

The values_t type alias and inner_t struct are correctly defined and necessary for handling node values and inner node data.


Line range hint 74-88:
Correct use of data_t union and impl_data_t struct.

The data_t union and impl_data_t struct are correctly defined and necessary for handling node data and implementation details.


94-108: Correct implementation of size calculation methods.

The sizeof_values_n, sizeof_collision_n, and sizeof_inner_n methods are correctly implemented to calculate the sizes of values, collision, and inner nodes.


111-149: Correct implementation of node access methods.

The kind, values, children, datamap, and nodemap methods are correctly implemented to access node kind, values, children, datamap, and nodemap.


153-192: Correct implementation of count and collision access methods.

The data_count, children_count, collision_count, and collisions methods are correctly implemented to count data, children, collisions, and access collision data.


195-219: Correct implementation of reference counting, ownership, and mutation checks.

The refs, ownee, and can_mutate methods are correctly implemented to handle reference counting, ownership, and mutation checks.


Line range hint 224-382:
Correct implementation of node creation and mutable values methods.

The make_inner_n, make_collision_n, and ensure_mutable_values methods are correctly implemented to create inner and collision nodes, and ensure mutable values.


383-481: Correct implementation of collision insertion and removal methods.

The copy_collision_insert, move_collision_insert, copy_collision_remove, and move_collision_remove methods are correctly implemented to handle collision insertion and removal.


483-552: Correct implementation of collision replacement, inner replacement, and ownership methods.

The copy_collision_replace, copy_inner_replace, owned, owned_values, and owned_values_safe methods are correctly implemented to handle collision replacement, inner replacement, and ownership.


554-788: Correct implementation of inner replacement and value replacement methods.

The copy_inner_replace_value, copy_inner_replace_merged, move_inner_replace_merged, copy_inner_replace_inline, and move_inner_replace_inline methods are correctly implemented to handle inner replacement and value replacement.


790-977: Correct implementation of inner value removal and insertion methods.

The copy_inner_remove_value, move_inner_remove_value, copy_inner_insert_value, and move_inner_insert_value methods are correctly implemented to handle inner value removal and insertion.


981-1040: Correct implementation of merged node creation methods.

The make_merged and make_merged_e methods are correctly implemented to handle merged node creation.


Line range hint 1041-1135:
Correct implementation of reference counting, node deletion, and deallocation methods.

The inc, dec, inc_nodes, delete_values, delete_inner, delete_collision, delete_deep, delete_deep_shift, deallocate_values, deallocate_collision, and deallocate_inner methods are correctly implemented to handle reference counting, node deletion, and deallocation.

src/immer/detail/rbts/rrbtree.hpp (20)

50-58: LGTM!

The empty_root method correctly creates an empty root node using std::aligned_storage_t for proper alignment.


60-67: LGTM!

The empty_tail method correctly creates an empty tail node using std::aligned_storage_t for proper alignment.


72-79: LGTM!

The from_initializer_list method correctly creates an rrbtree from an initializer list using an owner_t object.


80-88: LGTM!

The from_range method correctly creates an rrbtree from a range of iterators using an owner_t object and ensures compatibility with different iterator types.


93-100: LGTM!

The from_fill method correctly creates an rrbtree by filling it with a specified value using an owner_t object.


101-109: LGTM!

The default constructor correctly initializes the rrbtree with default values and asserts the tree's integrity.


110-117: LGTM!

The parameterized constructor correctly initializes the rrbtree with specified values and asserts the tree's integrity.


119-124: LGTM!

The copy constructor correctly initializes the rrbtree by copying another rrbtree and increments the reference count.


125-129: LGTM!

The move constructor correctly initializes the rrbtree by moving another rrbtree and swaps the contents with the other rrbtree.


Line range hint 131-137:
LGTM!

The copy assignment operator correctly uses the copy-and-swap idiom to assign the contents of another rrbtree.


138-143: LGTM!

The move assignment operator correctly uses the move-and-swap idiom to assign the contents of another rrbtree.


144-150: LGTM!

The swap method correctly uses std::swap to swap the contents of two rrbtree objects.


153-153: LGTM!

The destructor correctly uses the dec method to decrement the reference count of the root and tail nodes.


155-159: LGTM!

The inc method correctly increments the reference count of the root and tail nodes.


161-161: LGTM!

The dec method correctly uses the traverse method with a dec_visitor to decrement the reference count.


163-163: LGTM!

The tail_size method correctly calculates the size of the tail by subtracting the tail offset from the total size.


165-172: LGTM!

The tail_offset method correctly calculates the offset of the tail based on the root node's relaxed state and the total size.


Line range hint 174-189:
LGTM!

The traverse method correctly traverses the tree based on the tail offset and size, and applies the visitor to the nodes.


Line range hint 191-211:
LGTM!

The range-based traverse method correctly traverses the tree based on the tail offset and size, and applies the visitor to the nodes within the specified range.


Line range hint 213-223:
LGTM!

The traverse_p method correctly traverses the tree based on the tail offset and size, and applies the visitor to the nodes, returning a boolean value indicating success.

src/immer/detail/hamts/champ.hpp (25)

20-118: LGTM! The champ_debug_stats struct is well-structured.

The addition of champ_debug_stats is a useful enhancement for debugging purposes. The methods and friend operators are well-implemented.


Line range hint 130-184:
LGTM! The champ struct is well-defined.

The champ struct includes essential methods and member variables. The constructors, destructors, and operations are implemented correctly.


185-190: LGTM! The inc and dec methods handle reference counting correctly.

The methods ensure proper reference counting and handle node deletion appropriately.


193-248: LGTM! The do_check_champ and check_champ methods ensure data structure integrity.

The methods are well-implemented with detailed checks and appropriate use of recursive calls and bitwise operations.


250-280: LGTM! The do_get_debug_stats and get_debug_stats methods collect detailed debug statistics.

The methods are well-implemented with appropriate use of recursive calls and accumulation of statistics.


282-301: LGTM! The from_initializer_list and from_range methods provide convenient ways to create a champ.

The methods are well-implemented with appropriate use of initializer lists and iterators.


311-329: LGTM! The for_each_chunk and for_each_chunk_traversal methods provide a way to iterate over chunks.

The methods are well-implemented with appropriate use of recursive calls and function objects.


331-413: LGTM! The diff methods provide a way to calculate differences between two champ instances.

The methods are well-implemented with appropriate use of recursive calls and bitwise operations.


415-490: LGTM! The diff_data_node, diff_node_data, and diff_data_data methods handle specific cases of differences.

The methods are well-implemented with appropriate use of recursive calls and function objects.


492-525: LGTM! The diff_collisions method handles differences in collisions.

The method is well-implemented with appropriate use of nested loops and function objects.


Line range hint 537-556:
LGTM! The get method retrieves a value from the champ.

The method is well-implemented with appropriate use of recursive calls and bitwise operations.


559-622: LGTM! The add_result struct and do_add method handle adding a value to the champ.

The struct and method are well-implemented with appropriate use of recursive calls and bitwise operations.


623-628: LGTM! The add method handles adding a value to the champ.

The method is well-implemented with appropriate use of recursive calls and bitwise operations.


630-731: LGTM! The add_mut_result struct and do_add_mut method handle adding a value to the champ with mutation.

The struct and method are well-implemented with appropriate use of recursive calls and bitwise operations.


733-740: LGTM! The add_mut method handles adding a value to the champ with mutation.

The method is well-implemented with appropriate use of recursive calls and bitwise operations.


743-825: LGTM! The update_result struct and do_update method handle updating a value in the champ.

The struct and method are well-implemented with appropriate use of recursive calls and bitwise operations.


830-842: LGTM! The update method handles updating a value in the champ.

The method is well-implemented with appropriate use of recursive calls and bitwise operations.


844-897: LGTM! The do_update_if_exists method handles updating a value in the champ if it exists.

The method is well-implemented with appropriate use of recursive calls and bitwise operations.


899-910: LGTM! The update_if_exists method handles updating a value in the champ if it exists.

The method is well-implemented with appropriate use of recursive calls and bitwise operations.


912-1033: LGTM! The update_mut_result struct and do_update_mut method handle updating a value in the champ with mutation.

The struct and method are well-implemented with appropriate use of recursive calls and bitwise operations.


1035-1049: LGTM! The update_mut method handles updating a value in the champ with mutation.

The method is well-implemented with appropriate use of recursive calls and bitwise operations.


1051-1146: LGTM! The update_if_exists_mut_result struct and do_update_if_exists_mut method handle updating a value in the champ with mutation if it exists.

The struct and method are well-implemented with appropriate use of recursive calls and bitwise operations.


1149-1159: LGTM! The update_if_exists_mut method handles updating a value in the champ with mutation if it exists.

The method is well-implemented with appropriate use of recursive calls and bitwise operations.


1176-1194: LGTM! The sub_result struct handles the result of a subtraction operation.

The struct is well-implemented with appropriate use of union and enum.


Line range hint 1198-1269:
LGTM! The do_sub method handles performing a subtraction operation on the champ.

The method is well-implemented with appropriate use of recursive calls and bitwise operations.

src/immer/detail/rbts/position.hpp (26)

16-16: Include <utility> for std::forward

The addition of <utility> is appropriate for using std::forward in perfect forwarding.


41-43: Add methods node() and size() to empty_regular_pos

The new methods provide access to the node_ pointer and size information, consistent with existing methods.


46-52: Add each and each_pred methods to empty_regular_pos

The new methods provide iteration capabilities, consistent with the struct's purpose.


55-57: Add visit method to empty_regular_pos

The new method allows a visitor to interact with the struct, using std::forward for perfect forwarding.


74-76: Add methods node() and size() to empty_leaf_pos

The new methods provide access to the node_ pointer and size information, consistent with existing methods.


78-79: Add visit method to empty_leaf_pos

The new method allows a visitor to interact with the struct, using std::forward for perfect forwarding.


103-104: Add methods node() and size() to leaf_pos

The new methods provide access to the node_ pointer and size information, consistent with existing methods.


109-110: Add visit method to leaf_pos

The new method allows a visitor to interact with the struct, using std::forward for perfect forwarding.


135-136: Add methods node() and size() to leaf_sub_pos

The new methods provide access to the node_ pointer and size information, consistent with existing methods.


141-142: Add visit method to leaf_sub_pos

The new method allows a visitor to interact with the struct, using std::forward for perfect forwarding.


165-171: Add methods node() and descend to leaf_descent_pos

The new methods provide access to the node_ pointer and a placeholder for descent operation, consistent with existing methods.


173-174: Add visit method to leaf_descent_pos

The new method allows a visitor to interact with the struct, using std::forward for perfect forwarding.


197-198: Add methods node() and size() to full_leaf_pos

The new methods provide access to the node_ pointer and size information, consistent with existing methods.


203-204: Add visit method to full_leaf_pos

The new method allows a visitor to interact with the struct, using std::forward for perfect forwarding.


229-237: Add methods node(), size(), and this_size() to regular_pos

The new methods provide access to the node_ pointer, size information, and the size of the current node, consistent with existing methods.


241-243: Add each method to regular_pos

The new method provides iteration capabilities, calling each_regular for the actual implementation.


247-249: Add each_pred method to regular_pos

The new method provides predicate-based iteration capabilities, calling each_pred_regular for the actual implementation.


253-255: Add each_pred_zip method to regular_pos

The new method provides predicate-based iteration capabilities with another node, calling each_pred_zip_regular for the actual implementation.


259-261: Add each_pred_i method to regular_pos

The new method provides predicate-based iteration capabilities with index range, calling each_pred_i_regular for the actual implementation.


265-267: Add each_pred_right method to regular_pos

The new method provides predicate-based iteration capabilities from a starting index, calling each_pred_right_regular for the actual implementation.


271-273: Add each_pred_left method to regular_pos

The new method provides predicate-based iteration capabilities up to a specified count, calling each_pred_left_regular for the actual implementation.


277-279: Add each_i method to regular_pos

The new method provides iteration capabilities with index range, calling each_i_regular for the actual implementation.


283-285: Add each_right method to regular_pos

The new method provides iteration capabilities from a starting index, calling each_right_regular for the actual implementation.


289-291: Add each_left method to regular_pos

The new method provides iteration capabilities up to a specified count, calling each_left_regular for the actual implementation.


295-298: Add towards method to regular_pos

The new method provides navigation capabilities towards a specified index, calling towards_oh_ch_regular for the actual implementation.


301-306: Add towards_oh method to regular_pos

The new method provides navigation capabilities towards a specified index with an offset hint, calling towards_oh_ch_regular for the actual implementation.

src/immer/detail/arrays/no_capacity.hpp (6)

12-17: Include directives added.

The new include directives for <immer/config.hpp>, <cassert>, <cstddef>, and <stdexcept> are appropriate and necessary for the added functionality and improved exception handling.


35-38: Improved initialization in empty method.

The empty method now uses node_t::make_n(0) for initialization, ensuring correct initialization of an empty node.


Line range hint 79-102:
Destructor and data_mut method added.

The destructor ensures proper resource management by calling dec(). The data_mut method ensures data mutation is handled correctly.


104-123: Simplified from_range and from_fill methods.

The changes improve readability and exception handling in the from_range and from_fill methods.


150-150: Standardized exception handling in get_check method.

The get_check method now uses IMMER_THROW for exception handling, standardizing the approach.


164-198: Improved exception handling in push_back, assoc, and update methods.

The push_back, assoc, and update methods now use IMMER_TRY, IMMER_CATCH, and IMMER_RETHROW for improved exception handling.

src/immer/detail/iterator_facade.hpp (2)

22-54: Core iterator methods added in iterator_core_access.

The added methods for dereferencing, incrementing, decrementing, equality checking, advancing, and calculating distance provide essential functionality for iterator operations.


Line range hint 99-194:
Comprehensive iterator operations in iterator_facade.

The added methods for dereferencing, incrementing, decrementing, equality checking, and random access operations ensure comprehensive support for various iterator categories.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (2)
src/qt/lelantusdialog.cpp (2)

151-152: Signal-slot parameter mismatch in LelantusDialog

The slots updateGlobalFeeVariables and coinControlUpdateLabels do not accept any parameters, but they are connected to the signal idClicked(int) which expects an int parameter. This will cause a runtime error.

  • updateGlobalFeeVariables is defined as void updateGlobalFeeVariables()
  • coinControlUpdateLabels is defined as void coinControlUpdateLabels()

Please update the slots to accept an int parameter or connect them to a compatible signal.

Analysis chain

Verify signal-slot compatibility after connection update for groupCustomFee.

The connections for groupCustomFee have been updated to use idClicked instead of buttonClicked. Ensure that the connected slots (updateGlobalFeeVariables, coinControlUpdateLabels) are compatible with the new signal signature.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the compatibility of the signal `idClicked` with the connected slots in `LelantusDialog`.

# Test: Search for the method implementations in the file to ensure they match the expected signature.
rg --type cpp -A 5 $'void LelantusDialog::updateGlobalFeeVariables(int)' src/qt/lelantusdialog.cpp
rg --type cpp -A 5 $'void LelantusDialog::coinControlUpdateLabels(int)' src/qt/lelantusdialog.cpp

Length of output: 198


Script:

#!/bin/bash
# Description: Search for any definitions of `updateGlobalFeeVariables` and `coinControlUpdateLabels` in the codebase to verify their signatures.

# Search for the definition of `updateGlobalFeeVariables` in the codebase
rg --type cpp -A 5 'updateGlobalFeeVariables' 

# Search for the definition of `coinControlUpdateLabels` in the codebase
rg --type cpp -A 5 'coinControlUpdateLabels'

Length of output: 18423


148-150: Signal-slot compatibility issue detected.

The methods updateFeeSectionControls, updateGlobalFeeVariables, and coinControlUpdateLabels in LelantusDialog do not have the int parameter required by the idClicked(int) signal. This will result in a mismatch and the slots will not be called correctly.

  • void LelantusDialog::updateFeeSectionControls()
  • void LelantusDialog::updateGlobalFeeVariables()
  • void LelantusDialog::coinControlUpdateLabels()

Please update the method signatures to include the int parameter or adjust the signal connection accordingly.

Analysis chain

Verify signal-slot compatibility after connection update.

The connections for groupFee have been updated to use idClicked instead of buttonClicked. Ensure that the connected slots (updateFeeSectionControls, updateGlobalFeeVariables, coinControlUpdateLabels) are compatible with the new signal signature.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the compatibility of the signal `idClicked` with the connected slots in `LelantusDialog`.

# Test: Search for the method implementations in the file to ensure they match the expected signature.
rg --type cpp -A 5 $'void LelantusDialog::updateFeeSectionControls(int)' src/qt/lelantusdialog.cpp
rg --type cpp -A 5 $'void LelantusDialog::updateGlobalFeeVariables(int)' src/qt/lelantusdialog.cpp
rg --type cpp -A 5 $'void LelantusDialog::coinControlUpdateLabels(int)' src/qt/lelantusdialog.cpp

Length of output: 298


Script:

#!/bin/bash
# Description: Verify the compatibility of the signal `idClicked` with the connected slots in `LelantusDialog`.

# Test: Search for the method implementations in the entire repository to ensure they match the expected signature.
rg --type cpp -A 5 $'void LelantusDialog::updateFeeSectionControls(int)'
rg --type cpp -A 5 $'void LelantusDialog::updateGlobalFeeVariables(int)'
rg --type cpp -A 5 $'void LelantusDialog::coinControlUpdateLabels(int)'

Length of output: 220


Script:

#!/bin/bash
# Description: Verify the existence and signatures of the methods connected to the signal `idClicked` in `LelantusDialog`.

# Test: Search for the method implementations in the entire repository without specifying the parameter types.
rg --type cpp -A 5 $'void LelantusDialog::updateFeeSectionControls'
rg --type cpp -A 5 $'void LelantusDialog::updateGlobalFeeVariables'
rg --type cpp -A 5 $'void LelantusDialog::coinControlUpdateLabels'

Length of output: 1470

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9391c46 and 88b0aff.

Files selected for processing (22)
  • src/qt/addresstablemodel.cpp (4 hunks)
  • src/qt/bantablemodel.cpp (3 hunks)
  • src/qt/bitcoin.cpp (1 hunks)
  • src/qt/bitcoinamountfield.cpp (3 hunks)
  • src/qt/bitcoingui.cpp (1 hunks)
  • src/qt/guiutil.cpp (2 hunks)
  • src/qt/guiutil.h (1 hunks)
  • src/qt/lelantusdialog.cpp (2 hunks)
  • src/qt/notifymnemonic.cpp (1 hunks)
  • src/qt/optionsmodel.cpp (6 hunks)
  • src/qt/overviewpage.cpp (1 hunks)
  • src/qt/pcodemodel.cpp (2 hunks)
  • src/qt/peertablemodel.cpp (2 hunks)
  • src/qt/receivecoinsdialog.cpp (1 hunks)
  • src/qt/receiverequestdialog.cpp (3 hunks)
  • src/qt/recentrequeststablemodel.cpp (1 hunks)
  • src/qt/recentrequeststablemodel.h (1 hunks)
  • src/qt/sendcoinsdialog.cpp (2 hunks)
  • src/qt/splashscreen.h (1 hunks)
  • src/qt/transactiontablemodel.cpp (1 hunks)
  • src/qt/transactionview.cpp (4 hunks)
  • src/qt/utilitydialog.h (1 hunks)
Files skipped from review due to trivial changes (1)
  • src/qt/optionsmodel.cpp
Additional comments not posted (36)
src/qt/utilitydialog.h (1)

45-45: Approved: Constructor signature change in ShutdownWindow class.

The modification from Qt::WindowFlags f=0 to Qt::WindowFlags f=Qt::WindowFlags() is a best practice in C++, enhancing type safety and clarity.

src/qt/splashscreen.h (1)

24-24: Approved: Constructor signature update in SplashScreen class.

Changing the default value for Qt::WindowFlags f from 0 to Qt::WindowFlags() enhances type safety and code clarity, aligning with modern C++ best practices.

src/qt/notifymnemonic.cpp (1)

33-33: Approved: Simplification of translation function in NotifyMnemonic class.

Replacing trUtf8 with tr simplifies the translation process and aligns with standard practices in Qt, which supports UTF-8 by default in tr.

src/qt/recentrequeststablemodel.h (1)

85-85: Approved: Method signature update in RecentRequestsTableModel class.

Adding a default value of QModelIndex() to the parent parameter in the index method enhances usability by allowing simpler calls to this method, aligning with Qt model/view best practices.

src/qt/bantablemodel.cpp (3)

68-68: Approved change from qStableSort to std::stable_sort.

This change aligns with modern C++ standards and should maintain the intended functionality as long as the custom comparator BannedNodeLessThan supports it.


130-130: Approved change in date formatting method.

Using QLocale::system().toString for date formatting enhances localization support, making the application more adaptable to various regional settings.


152-152: Approved change in return value for invalid indices in flags method.

Returning Qt::ItemFlags() instead of 0 improves type safety and code clarity.

src/qt/pcodemodel.cpp (2)

132-132: Explicit return type in flags function.

The change from returning 0 to Qt::ItemFlags() is a good practice as it makes the return type explicit, which is safer and more readable.


248-248: Updated sorting algorithm to std::sort.

Switching to std::sort from qSort is a positive change as std::sort is more efficient and is the standard way in C++ to perform sorting. Ensure that sortPred is correctly defined and used.

src/qt/bitcoinamountfield.cpp (3)

10-10: Included guiutil.h for additional functionalities.

The inclusion of guiutil.h is necessary for the new method call GUIUtil::TextWidth used in the AmountSpinBox class. This change supports better modularity and code clarity.


106-106: Replaced BitcoinUnits::format with GUIUtil::TextWidth in minimumSizeHint.

This change likely aims to improve the performance or accuracy of width calculations. It's essential to verify that GUIUtil::TextWidth provides correct and efficient width calculations for UI elements.


178-178: Initialization of StepEnabled using default constructor.

Using StepEnabled() instead of 0 for initialization is a good practice as it avoids implicit conversions and makes the code type-safe and clearer.

src/qt/overviewpage.cpp (1)

477-477: Review: Adjustment to Standard Buttons in Dialog Constructor

The change from setStandardButtons(0) to setStandardButtons(StandardButtons()) alters how buttons are managed in the dialog. This change should ensure that no standard buttons are set unless explicitly specified elsewhere in the code. It's crucial to verify that this behavior is intended and that the dialog's functionality aligns with user expectations and UI consistency.

Verification successful

Review: Adjustment to Standard Buttons in Dialog Constructor

The change from setStandardButtons(0) to setStandardButtons(StandardButtons()) does not introduce any side effects. The dialog's functionality remains consistent with user expectations and UI consistency as no standard buttons are explicitly set or relied upon in the observed usage.

  • src/qt/overviewpage.cpp
  • src/qt/sendcoinsdialog.cpp
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify other usages of MigrateLelantusToSparkDialog to ensure consistency in button settings.

# Test: Search for other instances of MigrateLelantusToSparkDialog usage.
rg --type cpp "MigrateLelantusToSparkDialog" src/

Length of output: 1077


Script:

#!/bin/bash
# Description: Check the usage of MigrateLelantusToSparkDialog in src/qt/overviewpage.cpp and src/qt/sendcoinsdialog.cpp to verify standard button settings.

# Search for MigrateLelantusToSparkDialog instantiation and subsequent method calls in src/qt/overviewpage.cpp
rg -A 10 "MigrateLelantusToSparkDialog" src/qt/overviewpage.cpp

# Search for MigrateLelantusToSparkDialog instantiation and subsequent method calls in src/qt/sendcoinsdialog.cpp
rg -A 10 "MigrateLelantusToSparkDialog" src/qt/sendcoinsdialog.cpp

Length of output: 2582

src/qt/guiutil.cpp (5)

897-900: Review: TextWidth Calculation Update

The TextWidth function now uses horizontalAdvance instead of the deprecated width method from previous Qt versions. This update is crucial for correct text measurement in UI components, especially when dealing with variable-width fonts. Ensure this change is reflected everywhere TextWidth is used to avoid UI layout issues.

Verification successful

Review: TextWidth Calculation Update

The TextWidth function has been updated to use horizontalAdvance instead of the deprecated width method. This update is crucial for correct text measurement in UI components, especially when dealing with variable-width fonts. The search results confirm that the new TextWidth function is used correctly in the following files:

  • src/qt/sendcoinsdialog.cpp
  • src/qt/bitcoingui.cpp
  • src/qt/bitcoinamountfield.cpp
  • src/qt/lelantusdialog.cpp

No instances of the deprecated width method were found in these usages.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Ensure that TextWidth is used correctly with the new method.

# Test: Search for TextWidth usage in the project.
rg --type cpp "TextWidth" src/

Length of output: 1822


911-918: Review: HasPixmap Function Update

The HasPixmap function now supports different Qt versions for checking if a QLabel has a pixmap. This update is essential for ensuring that UI components are displayed correctly across various Qt versions. Testing across different environments is recommended to confirm that icons and images are handled correctly.

Verification successful

The HasPixmap function is used multiple times in src/qt/receiverequestdialog.cpp. To ensure that it is used consistently and correctly, I will check the context around its usage in this file.


Verification Successful: HasPixmap Function Usage

The HasPixmap function is used consistently and correctly within the src/qt/receiverequestdialog.cpp file. The function checks if a QLabel has a pixmap before performing actions, ensuring compatibility across different Qt versions.

  • mousePressEvent
  • saveImage
  • copyImage
  • contextMenuEvent
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that HasPixmap is used consistently and correctly across the project.

# Test: Search for HasPixmap usage in the project.
rg --type cpp "HasPixmap" src/

Length of output: 491


Script:

#!/bin/bash
# Description: Check the context around the usage of HasPixmap in src/qt/receiverequestdialog.cpp

# Display lines around each usage of HasPixmap
rg --type cpp --context 5 "HasPixmap" src/qt/receiverequestdialog.cpp

Length of output: 1003


920-931: Review: GetImage Function Implementation

The GetImage function retrieves an image from a QLabel, considering different Qt versions. This function is crucial for image manipulation tasks within the UI. Ensure that this function is used properly where image data is required from labels, and test across different Qt versions for consistency.

Verification successful

Verification Successful: Proper usage of GetImage function

The GetImage function is used appropriately within the QRImageWidget::exportImage method in src/qt/receiverequestdialog.cpp. The function is correctly utilized to retrieve image data from the widget, which is then used in the mousePressEvent method to set image data in a QMimeData object.

  • src/qt/receiverequestdialog.cpp: Verified usage in QRImageWidget::exportImage.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for proper usage of GetImage in the project.

# Test: Search for GetImage usage in the project.
rg --type cpp "GetImage" src/

Length of output: 212


Script:

#!/bin/bash
# Extract lines around the usage of GetImage in src/qt/receiverequestdialog.cpp to verify proper usage.
rg --type cpp -A 10 -B 10 "GetImage" src/qt/receiverequestdialog.cpp

Length of output: 799


902-909: Review: StartOfDay Function Enhancement

The StartOfDay function now handles different Qt versions to ensure compatibility. For Qt versions 5.14 and above, it uses startOfDay(), which is more precise and straightforward. This change improves the function's reliability across different Qt versions. Verify that this function's behavior remains consistent across different system settings and Qt versions.


93-93: Review: Updated dateTimeStr Function

The dateTimeStr function now uses QLocale::system() for formatting the date, which should adapt the date display to the user's system locale settings. This change enhances user experience by displaying dates in a familiar format. Ensure that all date usages throughout the application are tested to confirm they handle locale formats correctly.

Verification successful

Review: Updated dateTimeStr Function

The dateTimeStr function now uses QLocale::system() for formatting the date, which should adapt the date display to the user's system locale settings. This change enhances user experience by displaying dates in a familiar format. The usages of dateTimeStr across the codebase, such as in walletdb.cpp, recentrequeststablemodel.cpp, overviewpage.cpp, transactiontablemodel.cpp, transactiondesc.cpp, lelantuscoincontroldialog.cpp, and coincontroldialog.cpp, appear to be compatible with the new format. Ensure that all date usages throughout the application are tested to confirm they handle locale formats correctly.

  • src/qt/guiutil.cpp
  • src/wallet/walletdb.cpp
  • src/qt/recentrequeststablemodel.cpp
  • src/qt/overviewpage.cpp
  • src/qt/transactiontablemodel.cpp
  • src/qt/transactiondesc.cpp
  • src/qt/lelantuscoincontroldialog.cpp
  • src/qt/coincontroldialog.cpp
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other usages of dateTimeStr to ensure they handle the new format correctly.

# Test: Search for dateTimeStr usage in the project.
rg --type cpp "dateTimeStr" src/

Length of output: 1558


Script:

#!/bin/bash
# Description: Inspect the context around each usage of dateTimeStr to verify if they handle the new locale-aware format correctly.

# Extract lines around each usage of dateTimeStr to understand the context.
rg --type cpp "dateTimeStr" -A 5 -B 5 src/

Length of output: 9593

src/qt/bitcoin.cpp (1)

432-432: Review: Updated SplashScreen Initialization

The createSplashScreen method now explicitly uses Qt::WindowFlags() for initializing the SplashScreen object, which should ensure that no unintended window flags are set. This change is a good practice for clarity and avoiding potential issues with default parameters. Testing on different platforms is recommended to ensure that the splash screen behaves as expected.

Verification successful

Review: Updated SplashScreen Initialization

The createSplashScreen method now explicitly uses Qt::WindowFlags() for initializing the SplashScreen object, which should ensure that no unintended window flags are set. This change is a good practice for clarity and avoiding potential issues with default parameters. Testing on different platforms is recommended to ensure that the splash screen behaves as expected.

To verify the consistent behavior, we need to check the SplashScreen constructor implementation in src/qt/splashscreen.cpp.


Review: Updated SplashScreen Initialization

The createSplashScreen method now explicitly uses Qt::WindowFlags() for initializing the SplashScreen object, which should ensure that no unintended window flags are set. This change is a good practice for clarity and avoiding potential issues with default parameters. The SplashScreen constructor correctly handles the Qt::WindowFlags parameter, confirming that the change is appropriate.

  • File: src/qt/bitcoin.cpp
  • Lines: 432

Testing on different platforms is recommended to ensure that the splash screen behaves as expected.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistent behavior of the splash screen across different platforms.

# Test: Search for other instances where SplashScreen is used or tested.
rg --type cpp "SplashScreen" src/

Length of output: 2481


Script:

#!/bin/bash
# Description: Verify the SplashScreen constructor implementation to ensure it handles Qt::WindowFlags() properly.

# Extract the SplashScreen constructor implementation from src/qt/splashscreen.cpp
ast-grep --lang cpp --pattern $'class SplashScreen {
  $$$
  SplashScreen(const QPixmap &pixmap, Qt::WindowFlags f) : QSplashScreen(pixmap, f) {
    $$$
  }
  $$$
}' src/qt/splashscreen.cpp

Length of output: 535


Script:

#!/bin/bash
# Description: Verify the SplashScreen constructor implementation to ensure it handles Qt::WindowFlags() properly.

# Search for the SplashScreen constructor in src/qt/splashscreen.cpp
rg "SplashScreen::SplashScreen" src/qt/splashscreen.cpp -A 10

Length of output: 827

src/qt/lelantusdialog.cpp (1)

566-566: Review the updated indent setting method for fallbackFeeWarningLabel.

The method for setting the indent of fallbackFeeWarningLabel has been updated to use GUIUtil::TextWidth. Ensure that this utility method correctly calculates the width based on the font metrics and the specified text, and that it integrates well with the rest of the UI updates.

Verification successful

The updated method for setting the indent of fallbackFeeWarningLabel is correctly implemented.

The GUIUtil::TextWidth method uses QFontMetrics::horizontalAdvance to accurately calculate the width of the text, ensuring the indent is set appropriately. This change is valid and improves the consistency of the UI.

  • Implementation found in src/qt/guiutil.cpp:
    int TextWidth(const QFontMetrics& fm, const QString& text) {
        return fm.horizontalAdvance(text);
    }
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation and behavior of `GUIUtil::TextWidth`.

# Test: Search for the implementation of `GUIUtil::TextWidth` to ensure it behaves as expected.
rg --type cpp -A 10 $'int GUIUtil::TextWidth' src/qt/guiutil.cpp

Length of output: 65


Script:

#!/bin/bash
# Description: Locate the implementation of `GUIUtil::TextWidth` in the entire codebase.

# Step 1: Search for the declaration of `TextWidth` in header files.
rg --type cpp --type hpp -A 5 'TextWidth' 

# Step 2: Search for the implementation of `TextWidth` in source files.
rg --type cpp -A 10 'TextWidth'

Length of output: 7587

src/qt/transactionview.cpp (2)

293-316: Refactor date handling in chooseDate method.

The changes correctly utilize GUIUtil::StartOfDay to ensure that the date handling starts from the start of the day, which is a more precise way to handle date ranges in the UI. This should prevent any off-by-one errors or similar issues related to date boundaries.


665-666: Ensure consistent date range handling in dateRangeChanged.

The modifications to use GUIUtil::StartOfDay for both dateFrom and dateTo ensure that the date ranges are handled consistently and accurately, starting from the beginning of the day.

src/qt/addresstablemodel.cpp (3)

148-148: Approved change: Replaced qSort with std::sort.

The replacement of qSort with std::sort is a good move towards using standard C++ libraries, which are often more efficient and well-maintained.


154-156: Approved change: Replaced qLowerBound and qUpperBound with std::lower_bound and std::upper_bound.

Switching to standard C++ functions for bounds finding is a positive change, enhancing portability and potentially improving performance.

Also applies to: 208-210


466-466: Approved change: Updated return type for flags method.

Changing the return type from 0 to Qt::ItemFlags() in the flags method improves type safety and clarity, aligning with modern C++ practices.

Also applies to: 825-825

src/qt/bitcoingui.cpp (1)

1416-1416: Approved change: Use GUIUtil::TextWidth for calculating maximum width.

Using GUIUtil::TextWidth instead of fm.width is a more robust and potentially more accurate method for calculating text width, especially when dealing with different fonts and DPI settings.

src/qt/sendcoinsdialog.cpp (4)

1096-1096: Approve the change to setIndent using GUIUtil::TextWidth.

The update to use GUIUtil::TextWidth for setting indentation is a good practice as it encapsulates the width calculation, potentially improving maintainability and accuracy.

However, ensure that GUIUtil::TextWidth is implemented efficiently and correctly handles different scenarios.

Verification successful

Approve the change to setIndent using GUIUtil::TextWidth.

The update to use GUIUtil::TextWidth for setting indentation is a good practice as it encapsulates the width calculation, potentially improving maintainability and accuracy. The implementation of GUIUtil::TextWidth is efficient and correctly handles the calculation using QFontMetrics::horizontalAdvance.

  • src/qt/guiutil.cpp: int TextWidth(const QFontMetrics& fm, const QString& text) { return fm.horizontalAdvance(text); }
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the implementation of `GUIUtil::TextWidth`.
# Test: Search for the implementation of `GUIUtil::TextWidth` to ensure it's efficient and robust.
rg --type cpp 'TextWidth' -A 10

Length of output: 7517


195-195: Verify the new signal-slot connection for coinControlUpdateLabels.

The connection for coinControlUpdateLabels has been updated to use idClicked for both groupFee and groupCustomFee. Confirm that the behavior of idClicked matches the expected functionality of the previous buttonClicked signal.

Also applies to: 197-197


194-196: Verify the new signal-slot connection for updateGlobalFeeVariables.

The connection for updateGlobalFeeVariables has been updated to use idClicked for both groupFee and groupCustomFee. Confirm that the behavior of idClicked matches the expected functionality of the previous buttonClicked signal.

Also applies to: 197-197


193-193: Verify the new signal-slot connection for updateFeeSectionControls.

The connection has been changed to use idClicked instead of buttonClicked. Ensure that idClicked is triggered in the same scenarios and carries the same data type as the previous signal to maintain functionality.

src/qt/peertablemodel.cpp (2)

89-89: Approved change from qStableSort to std::stable_sort.

Switching to std::stable_sort aligns with modern C++ standards and ensures better compatibility and performance across different compilers and platforms.


200-200: Improvement in returning Qt::ItemFlags() instead of 0.

Returning Qt::ItemFlags() instead of a plain 0 enhances type safety and readability, making the code more idiomatic and less prone to errors related to type mismatches.

src/qt/receiverequestdialog.cpp (3)

46-46: Approved use of GUIUtil::GetImage for image export.

Using a centralized utility function for image handling simplifies the code and promotes reuse, which is beneficial for maintainability and potential future updates.


51-51: Approved use of GUIUtil::HasPixmap in mouse event handling.

Centralizing the check for pixmap existence through GUIUtil::HasPixmap helps in maintaining consistency and potentially reduces errors by abstracting the pixmap-related logic.


67-67: Approved use of GUIUtil::HasPixmap for image operations.

Replacing direct pixmap checks with GUIUtil::HasPixmap in methods like saveImage, copyImage, and during context menu events ensures a consistent approach to handling image availability checks.

Also applies to: 78-78, 85-85

src/qt/recentrequeststablemodel.cpp (1)

223-223: Approved change from qSort to std::sort.

This change is consistent with modern C++ practices and ensures better performance and compatibility across different platforms and compilers.

@@ -17,6 +17,7 @@
#include "transactiontablemodel.h"
#include "walletmodel.h"
#include "pcodemodel.h"
#include "guiutil.h"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Header inclusion of guiutil.h is redundant.

The header guiutil.h is included twice in this file. Redundant includes can lead to confusion and clutter in the codebase.

-#include "guiutil.h"

Committable suggestion was skipped due to low confidence.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 88b0aff and ab6b99a.

Files selected for processing (22)
  • src/qt/addresstablemodel.cpp (4 hunks)
  • src/qt/bantablemodel.cpp (3 hunks)
  • src/qt/bitcoin.cpp (1 hunks)
  • src/qt/bitcoinamountfield.cpp (3 hunks)
  • src/qt/bitcoingui.cpp (1 hunks)
  • src/qt/guiutil.cpp (2 hunks)
  • src/qt/guiutil.h (1 hunks)
  • src/qt/lelantusdialog.cpp (2 hunks)
  • src/qt/notifymnemonic.cpp (1 hunks)
  • src/qt/optionsmodel.cpp (6 hunks)
  • src/qt/overviewpage.cpp (1 hunks)
  • src/qt/pcodemodel.cpp (2 hunks)
  • src/qt/peertablemodel.cpp (2 hunks)
  • src/qt/receivecoinsdialog.cpp (1 hunks)
  • src/qt/receiverequestdialog.cpp (3 hunks)
  • src/qt/recentrequeststablemodel.cpp (1 hunks)
  • src/qt/recentrequeststablemodel.h (1 hunks)
  • src/qt/sendcoinsdialog.cpp (2 hunks)
  • src/qt/splashscreen.h (1 hunks)
  • src/qt/transactiontablemodel.cpp (1 hunks)
  • src/qt/transactionview.cpp (3 hunks)
  • src/qt/utilitydialog.h (1 hunks)
Files skipped from review as they are similar to previous changes (22)
  • src/qt/addresstablemodel.cpp
  • src/qt/bantablemodel.cpp
  • src/qt/bitcoin.cpp
  • src/qt/bitcoinamountfield.cpp
  • src/qt/bitcoingui.cpp
  • src/qt/guiutil.cpp
  • src/qt/guiutil.h
  • src/qt/lelantusdialog.cpp
  • src/qt/notifymnemonic.cpp
  • src/qt/optionsmodel.cpp
  • src/qt/overviewpage.cpp
  • src/qt/pcodemodel.cpp
  • src/qt/peertablemodel.cpp
  • src/qt/receivecoinsdialog.cpp
  • src/qt/receiverequestdialog.cpp
  • src/qt/recentrequeststablemodel.cpp
  • src/qt/recentrequeststablemodel.h
  • src/qt/sendcoinsdialog.cpp
  • src/qt/splashscreen.h
  • src/qt/transactiontablemodel.cpp
  • src/qt/transactionview.cpp
  • src/qt/utilitydialog.h

Copy link
Contributor

@levonpetrosyan93 levonpetrosyan93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@justanwar
Copy link
Member

Except for EXCEPTION_ACCESS_VIOLATION when starting Qt wallet with -disablewallet flag (not that you should), everything looking good so far.

@reubenyap reubenyap merged commit 5a1bb0b into master Aug 17, 2024
6 checks passed
@reubenyap reubenyap deleted the fix-warnings branch August 17, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants