Substring expansion can evaluate the position expression twice
perfloop-oss/cpp2rust · UNCATALOGUED CORRECTNESS
https://perfloop.ai/t/oss/case_gs6rt5cvpd
Verdict
VERIFIED · settled 2026-11-01 · merged as Cpp2Rust/cpp2rust#577
What happened: The assertion is violated on the comparison and satisfied with this change.
Hypothesis
The substring translation repeats its position operand in the generated expression. A position expression that increments a counter may therefore change both the returned bytes and caller state. Source inspection supports this separate semantic risk; this specific call has not been run through the complete translator.
The f1 template uses a1 twice, for the slice start and endpoint, and a0 twice, for the source and its length. These are translation placeholders, not ordinary function parameters evaluated before entry: docs/src/rules/writing-rules.md explains that rule bodies are spliced inline. Converter::ConvertIRFragment substitutes each occurrence separately. ConvertCallExpr and CollectRefBindingTempArgs only arrange reference-binding temporaries here, and NeedsRefBindingTemp immediately excludes the scalar position parameter.
A supported trigger is std::string s = "hello"; std::size_t pos = 1; auto out = s.substr(pos++, 2). C++ evaluates the increment once, returns "el", and leaves pos equal to 2. Direct substitution into the current rule predicts two increments and a different endpoint. This does not depend on overflow, invalid positions, concurrency or an imagined mutable actor. How often consumers pass such expressions is unknown.
The smallest useful Case check is to translate that fixture and compare both the substring bytes and final counter with C++. Inspect the emitted bindings as well as executing the result. If an earlier transformation already captures the scalar expression once, that rejects this lead. Otherwise, verify evaluation-once after the repair and retain the existing source-borrow and independent-result behavior. The work should remain local unless shared argument materialization proves both necessary and compatible.
Reviewed upstream implementations in https://github.com/Cpp2Rust/cpp2rust/pull/571 and https://github.com/Cpp2Rust/cpp2rust/pull/572 do not add evaluation-once handling. https://github.com/Cpp2Rust/cpp2rust/pull/573 adjusts operand matching for vector<bool>, not repeated placeholder evaluation. No equivalent substring repair appeared in the refreshed open PR set.
Change to test: Establish evaluation-once behavior for the substring receiver and arguments before using them to construct the range. Use scoped bindings or argument materialization without cloning the whole source or extending a source borrow across argument side effects.
Where it lives
Evidence
The assertion is violated on the comparison and satisfied with this change: `For the hello substring fixtures in tests/unit/string.cpp, both translation models evaluate the receiver, position and count operands once: the position increment returns el with counter 2, the count increment returns el with counter 3, and the counted receiver returns el with counter 1.`
Checks: 4 of 4 passed. Verification: no defect found.