Files that changed from the base of the PR and between 55bac8e and f33dfe6.

• src/crab/wto.cpp (8 hunks)
• src/crab/wto.hpp (3 hunks)
• src/crab/wto_cycle.hpp (0 hunks)
• src/crab/wto_nesting.hpp (0 hunks)

• src/crab/wto_cycle.hpp
• src/crab/wto_nesting.hpp

src/crab/wto.hpp (1)

51-51: Confirm Availability of 'std::ranges::reverse_view'

The code uses std::ranges::reverse_view in the operator<< overload for wto_nesting_t. This requires C++20 support. Please verify that the project is configured to use C++20 standard or replace with an alternative approach compatible with the project's C++ standard.

src/crab/wto.cpp (4)

8-10: LGTM!

The added comments enhance clarity by explaining how _visit_stack avoids stack overflows compared to the recursive implementation.

---

11-27: Implementation of operator> is correct and efficient

The operator> function for wto_nesting_t correctly compares nesting levels to determine if one nesting is a superset of another. The logic efficiently handles the comparisons starting from the outermost nesting.

---

30-30: Passing containing_cycle as a const std::weak_ptr improves performance

Changing the containing_cycle parameter to a const std::weak_ptr<wto_cycle_t>& avoids unnecessary copies and potential overhead, improving performance.

Also applies to: 55-55, 87-87, 112-112

---

39-39: Use of emplace enhances efficiency

Replacing push with emplace in _visit_stack operations constructs the objects in place, eliminating unnecessary copies and improving performance.

Also applies to: 43-43

Files that changed from the base of the PR and between f33dfe6 and 7032f13.

• src/crab/fwd_analyzer.cpp (7 hunks)
• src/crab/thresholds.cpp (2 hunks)
• src/crab/thresholds.hpp (1 hunks)
• src/crab/wto.cpp (6 hunks)
• src/crab/wto.hpp (2 hunks)

src/crab/thresholds.hpp (1)

68-68: Improved const-correctness in operator() method

This change enhances the method's const-correctness by modifying the parameter type from std::shared_ptr<wto_cycle_t>& to const std::shared_ptr<wto_cycle_t>&. This is a positive improvement for several reasons:

1. It clearly communicates that the method doesn't modify the shared_ptr itself.
2. It allows the method to be called with both const and non-const shared_ptr objects, increasing its flexibility.
3. It adheres to the C++ best practice of accepting parameters by const reference when modification isn't needed.
4. The change is backwards compatible and doesn't affect the ability to modify the pointed-to object if necessary.

This modification contributes to a more robust and maintainable codebase by enforcing const-correctness.

src/crab/thresholds.cpp (3)

71-71: Approve: Improved parameter type for better const-correctness

The change from std::shared_ptr<wto_cycle_t>& to const std::shared_ptr<wto_cycle_t>& is a good practice. It clearly indicates that the method won't modify the shared pointer itself, enhancing type safety and making the code's intent more explicit.

---

Line range hint 71-88: Summary: Changes align with PR objectives

The modifications in this file, particularly in the wto_thresholds_t::operator() method, are consistent with the PR's objectives of cleaning up the WTO (Well-Founded Ordering) algorithm implementation. The changes improve const-correctness and suggest a restructuring of how components are handled, which aligns with the goal of removing the recursive version and enhancing code organization.

---

88-88: Verify the impact of removing dereferencing in std::visit

The change from std::visit(*this, *component) to std::visit(*this, component) suggests a modification in how components are stored or accessed. This could be related to changes in the wto_cycle_t class or its container type.

Please run the following script to check for any related changes in the wto_cycle_t class and its usage:

✅ Verification successful

Impact of Removing Dereferencing in std::visit Verified

The change from std::visit(*this, *component) to std::visit(*this, component) in src/crab/thresholds.cpp:88 is isolated and does not affect other parts of the codebase. No related changes were found in the wto_cycle_t class or its usage elsewhere.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for changes in wto_cycle_t class and its usage

# Search for wto_cycle_t definition
echo "wto_cycle_t definition:"
rg --type cpp "class\s+wto_cycle_t" -A 10

# Search for wto_cycle_t usage in containers
echo "\nwto_cycle_t usage in containers:"
rg --type cpp "(?:vector|list|set)<.*wto_cycle_t.*>"

# Search for other occurrences of std::visit with wto_cycle_t
echo "\nOther occurrences of std::visit with wto_cycle_t:"
rg --type cpp "std::visit.*wto_cycle_t"

Length of output: 1303

---

Script:

#!/bin/bash
# Description: Further check for std::visit usages involving wto_cycle_t through type aliases and indirect references

# Search for std::visit usages without specifying wto_cycle_t directly
echo "std::visit usages without direct wto_cycle_t reference:"
rg --type cpp "std::visit\s*\(.*\wto_cycle_t.*\)"

# Search for type aliases related to wto_cycle_t and their usage with std::visit
echo "\nType aliases related to wto_cycle_t used with std::visit:"
rg --type cpp "using\s+\w+\s+=\s+.*wto_cycle_t.*" | while read -r alias; do
    alias_name=$(echo "$alias" | awk '{print $2}')
    rg --type cpp "std::visit\s*\(.*$alias_name.*\)"
done

# Search for std::visit in files that include wto_cycle_t definition
echo "\nstd::visit usages in files including wto_cycle_t definition:"
rg --type cpp -l "wto_cycle_t" | xargs rg --type cpp "std::visit"

Length of output: 1254

src/crab/fwd_analyzer.cpp (6)

30-30: Improved const-correctness in member_component_visitor::operator()

The change from std::shared_ptr<wto_cycle_t>& to const std::shared_ptr<wto_cycle_t>& improves const-correctness. This is a good practice in C++ as it clearly communicates that the method doesn't modify the shared_ptr itself.

---

38-38: Consistent update in std::visit call

The change from std::visit(*this, *component) to std::visit(*this, component) is consistent with the updated method signature. Since component is now a reference to a shared_ptr, it doesn't need to be dereferenced before passing to std::visit. This change maintains the correct behavior while simplifying the code.

---

117-117: Consistent const-correctness improvement in interleaved_fwd_fixpoint_iterator_t::operator()

Similar to the change in member_component_visitor, this update from std::shared_ptr<wto_cycle_t>& to const std::shared_ptr<wto_cycle_t>& improves const-correctness. This consistent application of const-correctness across the codebase is a good practice that enhances code quality and maintainability.

---

128-129: Improved memory management with std::shared_ptr

The changes from raw pointers to std::shared_ptr for wto_cycle_t are a significant improvement:

1. Line 128: Using std::holds_alternative<std::shared_ptr<wto_cycle_t>> instead of std::holds_alternative<wto_cycle_t*>.
2. Line 129: Using std::get<std::shared_ptr<wto_cycle_t>> instead of std::get<wto_cycle_t*>.

These changes align with modern C++ best practices by leveraging smart pointers. This can help prevent memory leaks, improve resource management, and make ownership semantics clearer. Good job on this refactoring!

---

193-194: Refactoring to use std::variant and std::visit

The changes in the interleaved_fwd_fixpoint_iterator_t::operator() method (lines 193-194 and 213-214) reflect a shift towards using std::variant and std::visit:

1. The condition !std::holds_alternative<label_t>(component) || (std::get<label_t>(component) != head) checks if the component is not a label_t or if it's a label_t but not equal to the head.
2. The use of std::visit(*this, component) is consistent with the changes seen earlier in the file.

These changes likely replace a custom polymorphic approach with the more type-safe and efficient std::variant and std::visit. This refactoring can lead to more robust and easier to maintain code, as it leverages the type system to handle different component types safely.

Also applies to: 213-214

---

Line range hint 1-231: Overall assessment of changes

The changes in this file represent a significant improvement in code quality and adherence to modern C++ best practices:

1. Improved const-correctness throughout the file.
2. Shift from raw pointers to std::shared_ptr for better memory management.
3. Refactoring to use std::variant and std::visit for type-safe handling of heterogeneous collections.

These changes should lead to more robust, maintainable, and efficient code. They appear to be part of a larger effort to modernize the codebase, which is commendable. The consistent application of these improvements across different parts of the file is particularly noteworthy.

Great job on this refactoring effort!

Files that changed from the base of the PR and between 7032f13 and 158e6e4.

• src/crab/wto.cpp (6 hunks)
• src/crab/wto.hpp (2 hunks)

src/crab/wto.hpp (3)

36-47: Well-Structured Implementation of wto_nesting_t Class

The wto_nesting_t class is effectively designed to manage the collection of heads of nested components. The use of reverse order storage for optimizing insertion performance is a thoughtful approach. The constructor correctly utilizes move semantics to efficiently initialize _heads.

---

67-93: Robust Handling in wto_cycle_t Class

The wto_cycle_t class appropriately encapsulates cycles within the weak topological ordering. The head() method includes necessary checks for empty components and safely accesses the variant, preventing potential runtime errors. The use of [[nodiscard]] attributes for the iterators enhances code correctness by encouraging the handling of return values.

---

114-127: Appropriate Usage of Constructors and Iterators in wto_t Class

The wto_t class constructor is correctly marked as explicit, preventing unintended implicit conversions. The iterator methods begin() and end() are properly implemented, providing const-correct reverse iterators over the components. The inclusion of the nesting() method is well-placed for accessing the nesting information of a label.

src/crab/wto.cpp (9)

13-29: Implementation of operator> is correct and efficient

The operator> overload for wto_nesting_t correctly compares nesting structures by size and content, iterating from the outermost elements inward. The logic is sound and efficiently implemented.

---

68-73: Consistent use of const references for parameters

The functions push_successors, start_visit, and continue_visit now correctly use const std::weak_ptr<wto_cycle_t>& for the containing_cycle parameter, improving performance by avoiding unnecessary copies.

---

Line range hint 121-140: Efficient creation of shared pointers

The use of std::make_shared to create new cycles with containing_cycle is efficient and avoids unnecessary copies. The constructor parameters are correctly forwarded.

---

227-232: Usage of C++20 <ranges> library

The use of std::ranges::reverse_view provides a clean and efficient way to iterate in reverse. Ensure that the project's build configuration supports C++20 standards to prevent any compilation issues.

---

276-282: Correct implementation of collect_heads function

The collect_heads function effectively gathers all the heads of nested components containing a given label, handling optional values appropriately without risk of infinite loops.

---

286-293: Acknowledging the unused nesting function

As noted by static analysis tools, the nesting function is currently unused. Since this was previously flagged, no additional action is required unless new developments necessitate its usage.

🧰 Tools

🪛 cppcheck

[style] 286-286: The function 'nesting' is never used.

(unusedFunction)

---

242-245: Overload of operator<< for wto_t

The operator<< overload for wto_t enhances debug capabilities by allowing easy printing of wto_t objects. The implementation is appropriate and adheres to best practices.

---

210-225: Correct use of visitor pattern in print_visitor

The print_visitor class effectively implements the visitor pattern to handle different component types. The overloads are correctly defined, and shared pointers are safely dereferenced after null checks.

---

253-272: Proper handling of optional values in head method

The head method correctly navigates the nested cycles to find the encompassing head label, handling std::optional and weak pointers safely to prevent dereferencing null or expired pointers.