[clang-tools-extra] [clang-tidy] Add modernize-use-std-tie check (PR #191218)

[Daniil Dudkin via cfe-commits]

https://github.com/unterumarmung commented:

This is a useful prototype, but in its current form the implementation is too fragile for a clang-tidy check.

The main concern is that the check matches every `operator<` / `operator>` and then tries to recover intent by scraping raw source text from all `return` statements in the body. That is far too permissive. The code never actually proves that the function implements a lexicographical comparison, so it can rewrite bodies with unrelated early returns, extra control flow, side effects, mismatched fields, or mixed comparison directions. More generally, the transformation is driven by textual heuristics (`find('<')`, `find('>')`, `lhs.` prefix checks, special-casing `()`), not by AST structure, which makes it brittle for anything beyond the exact spelling in the test: different parameter names, member operators using `this`, nested expressions, casts, dereferences, templates, macros, and other ordinary C++ syntax variations.

There are also fix-it correctness issues. The implementation assumes the RHS can be reconstructed by mirroring the extracted LHS text, which is not guaranteed. It does not guard against duplicate synthesized temporaries or name collisions, and it only handles one very specific kind of computed expression (`lhs.foo()`). That means even some cases it intends to support can easily produce invalid or semantically different code.

I think this needs to be AST-driven instead. In practice, the matcher should recognize a much narrower body shape up front, prove that each clause is of the expected `if (lhs.k != rhs.k) return lhs.k < rhs.k;` / final-return form, and then build the replacement from the matched expressions rather than token slicing. Until that happens, I do not think this is safe to ship as an automatic rewrite.

On the test side, the current coverage is much too narrow for a check like this. Right now the tests only cover one happy-path shape: a free `operator<` / `operator>` written as an `if` / `else if` chain, comparing direct members plus one repeated nullary member call hoisted into temporaries. That is a fine smoke test, but it leaves most of the interesting surface area untested.

We should have broader positive coverage for equivalent shapes, such as sequential `if` statements, longer comparison chains, member operators, hidden friends, out-of-line definitions, and variants with `constexpr`, attributes, or trailing return types. It would also be useful to cover a wider range of key expressions: nested member access, subscripts, dereferences, casts, getters, and other cases where the fix may need to preserve evaluation semantics or introduce temporaries.

Just as importantly, the suite needs substantially more negative coverage. For a rewriting check, that matters as much as the positive path. There should be tests showing that we do not transform near-miss cases with extra control flow, side effects, mismatched compared expressions, mixed ordering directions, non-strict operators, custom equality conditions, boolean-formula implementations, macro-generated code, and dependent/template contexts that are not provably safe. It would also be good to exercise interaction with modern C++ features such as templates, concepts, macros, modules, and existing `<=>` / defaulted comparisons.

As it stands, the tests show that one hand-written example works; they do not yet demonstrate that the matcher is precise or that the rewrite is robust across the range of C++ code this check will encounter.



https://github.com/llvm/llvm-project/pull/191218