Cleanup wto parts in fwd_analyzer

[elazarg]

• Make nesting field in wto_t mutable (cache)
• Move component member check to wto.cpp and simplify
• Make extrapolate and refine functions file-static instead of class-static

Summary by CodeRabbit

• New Features
  • Introduced a new function, is_component_member, for checking membership in components.
• Improvements
  • Enhanced const-correctness for several methods in the wto_t class, allowing for safer use in const contexts.
  • Simplified control flow in the forward analyzer by removing the visitor pattern and improving modularity.
• Bug Fixes
  • Updated method signatures to reflect const-correctness, ensuring methods do not modify object state.

[coderabbitai]

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Reviewing files that changed from the base of the PR and between 5195871 and cb571ad.

📒 Files selected for processing (3)

• src/crab/fwd_analyzer.cpp (6 hunks)
• src/crab/wto.cpp (4 hunks)
• src/crab/wto.hpp (3 hunks)

🧰 Additional context used

🪛 cppcheck (2.10-2)

src/crab/wto.cpp

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

(unusedFunction)

🔇 Additional comments (10)

src/crab/wto.hpp (3)

95-96: Function declaration looks good.

Clear purpose, appropriate parameters and return type.

---

110-111: Const-correctness improvement looks good.

Methods don't modify object state, making them const is appropriate.

---

108-108: Verify thread safety of mutable cache.

The mutable cache is appropriate for performance optimization, but ensure thread safety if the class is used in concurrent contexts.

✅ Verification successful

Thread safety not a concern for this mutable cache

The mutable cache is used in a const member function nesting() for performance optimization. The concurrent test cases show that WTO instances are not shared between threads - each thread operates on its own separate WTO instance. The mutable cache is thread-safe by design through this non-shared usage pattern.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for concurrent usage patterns
rg -A 5 "std::thread|std::async|parallel|concurrent" 

Length of output: 849

---

Script:

#!/bin/bash
# Check if the WTO class is used in test_verify.cpp
ast-grep --pattern 'class wto'

# Check the test_analyze_thread function implementation
ast-grep --pattern 'test_analyze_thread'

# Check for any mutex or lock usage around WTO
rg -A 3 "mutex|lock.*wto"

Length of output: 772

---

Script:

#!/bin/bash
# Check the implementation of test_analyze_thread to see if it uses WTO
rg -A 10 "^void test_analyze_thread"

# Check if WTO is used in concurrent contexts
rg -l "wto.*" src/test/test_verify.cpp

Length of output: 825

---

Script:

#!/bin/bash
# Check if WTO is used in verify function
ast-grep --pattern 'verify($$$) {
  $$$
}'

# Look for WTO usage in verify
rg -A 5 "wto.*" src/crab/wto.hpp

Length of output: 3900

src/crab/wto.cpp (3)

Line range hint 268-290: LGTM

Proper const qualification for read-only method.

---

Line range hint 292-299: LGTM

Proper const qualification for read-only method.

---

Line range hint 302-308: Verify usage of nesting method

Static analysis reports this method as unused. Verify if this is a false positive.

🧰 Tools

🪛 cppcheck (2.10-2)

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

(unusedFunction)

src/crab/fwd_analyzer.cpp (4)

18-18: Const correctness of _wto member

Declaring _wto as const wto_t enhances immutability and ensures that the well-known topological order remains unmodified after initialization. This change aligns with the intention of caching within wto_t using mutable members.

---

33-35: Adding const to accessor methods

Marking get_pre and get_post as const methods is appropriate since they do not modify the state of the interleaved_fwd_fixpoint_iterator_t instance. This change improves const-correctness and allows these methods to be called on const instances.

---

51-51: Const correctness of join_all_prevs method

Making join_all_prevs a const method is consistent with its purpose of aggregating predecessor states without altering the object's state.

---

134-136: Verify logic for skipping cycles

The assignment _skip = !entry_in_this_cycle; determines whether to skip analyzing the current cycle. Verify that this logic correctly handles scenarios where entry_label is within nested components of the cycle.

Run the following script to ensure that entry_label membership is accurately detected in nested components:

✅ Verification successful

Logic for skipping cycles is correctly implemented

The implementation in src/crab/wto.cpp shows that is_component_member recursively checks both direct and nested components:

• Direct check: Matches if label equals component label
• Nested check: Recursively searches through sub-components

The skip logic in fwd_analyzer.cpp correctly uses this function to determine if entry_label is present in any level of the cycle's hierarchy before deciding to skip analysis.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if entry_label is correctly identified as a member of any component in the cycle.

# Test: Search for occurrences of entry_label within the cycle components.
# Expect: Should return at least one match if entry_label is a member.

ast-grep --pattern $'is_component_member($_, $_)'

Length of output: 50

---

Script:

#!/bin/bash
# Search for the implementation of is_component_member function
rg -U "is_component_member.*\{[\s\S]*?\}" -g "*.{cpp,hpp}"

# Search for any tests related to cycle analysis and component membership
rg -l "test.*cycle|test.*component" -g "*test*.{cpp,hpp}"

# Find all files containing cycle-related analysis
rg -l "cycle|component" -g "*.{cpp,hpp}"

Length of output: 756

---

Script:

#!/bin/bash
# Get the implementation of the cycle class and its usage in WTO
rg -U "class\s+cycle" -g "*.hpp" -A 10

# Get the context around the skip logic in fwd_analyzer
rg -U "entry_in_this_cycle" -g "fwd_analyzer.cpp" -B 5 -A 5

# Look for any WTO-related tests
rg -U "TEST.*WTO" -g "*test*.cpp" -A 20

Length of output: 1610

[coveralls]

coverage: 89.769% (+0.09%) from 89.684%
when pulling cb571ad on const-wto
into 5195871 on main.