| Issue |
172930
|
| Summary |
[clang-tidy] Check request: bugprone-mutable-predicate-state
|
| Labels |
clang-tidy
|
| Assignees |
|
| Reporter |
denzor200
|
**Problem Statement**
C++ algorithms may freely copy predicate objects and do not guarantee evaluation order. This creates two constraints:
1. **Non-Guaranteed Identity**: Algorithm copies break assumptions about predicate object identity and state continuity.
2. **Non-Guaranteed Evaluation Order**: Unspecified visitation order prevents reliable use of invocation counts or sequences.
Predicates with external side effects violate second constraint and with mutable internal state violate both constraints, leading to undefined behavior and breaking [**CTR58-CPP. Predicate function objects should not be mutable**](https://wiki.sei.cmu.edu/confluence/spaces/cplusplus/pages/88046679/CTR58-CPP.+Predicate+function+objects+should+not+be+mutable). In addition, external side effects can lead to unexpected consequences, as you will see in the following paragraphs.
**Proposed Checker**
`bugprone-mutable-predicate-state` should detect dangerous state modifications in callables passed to standard algorithms and LLVM utilities.
**Detection Scope**
Targets callables passed to:
- `<algorithm>` functions (`std::sort`, `std::remove_if`, etc.)
- `std::erase_if` container methods
- LLVM utilities with similar semantics (`llvm::search`)
Flags callables that **modify state**:
- Lambdas: Modify variables captured by value (with `mutable`) or by reference (`[&]`)
- Functors: Modify non-static data members and other external data in `operator()`
**Examples from Practice**
1. Example 1: This was encountered in production code where sorting unexpectedly populated a lookup map.
```cpp
std::vector<std::shared_ptr<Cookie>> cookies_list;
std::map<int, int> filter; // Some existing data
// Each lambda call uses operator[] on `filter`, which INSERTs a default element for a new key,
// affecting another code that uses `filter`
cookies_list.sort([&](const auto& a, const auto& b) {
return filter[a->id] < filter[b->id]; // MODIFIES `filter` via operator[]
});
```
2. Example 2: This pattern was found in an LLVM/Clang patch (see discussion in [PR #169965](https://github.com/llvm/llvm-project/pull/169965#discussion_r2603583908)).
```cpp
// Simplified example inspired by the LLVM code review.
BoundNodesTreeBuilder CurrentBuilder;
const auto Found = llvm::search(
CS->body(), Matchers,
[&](const Stmt *StmtPtr, const Matcher<Stmt> &Matcher) mutable { // 'mutable' is a smell
BoundNodesTreeBuilder StepBuilder;
StepBuilder.addMatch(CurrentBuilder); // Reads captured state
// ... logic that may change StepBuilder ...
CurrentBuilder = StepBuilder; // MODIFIES captured state 'CurrentBuilder'
return true;
});
```
**Why This is Valuable**
* **Catches Subtle Bugs**: These bugs are non-obvious, often surface only with specific standard library implementations or optimization levels, and are hard to debug.
* **Enforces Best Practices**: Guides developers towards pure function predicates and explicit state management (e.g., using `std::reference_wrapper`, etc).
* **Prevents Real Issues**: As shown, this pattern has caused problems in real-world LLVM development and application code.
This checker would significantly improve code reliability by detecting this class of logic errors at compile time.
![]()
_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
