https://github.com/unterumarmung requested changes to this pull request.
This patch does not look ready for review yet. The current implementation
misses basic constraints, and the tests do not exercise the cases needed to
show that the fix is safe. In particular, the matcher does not restrict the
call to `insert`, and the fix-it assumes properties about both the range and
destination expressions that are not generally true.
A few examples that break:
```cpp id="m4k3s9"
for (int I : In)
Out.erase(I);
```
```cpp id="k2j8qa"
for (int I : makeSet())
Out.insert(I);
// becomes
Out.insert(makeSet().begin(), makeSet().end());
```
```cpp id="a8mz7c"
for (int I : In)
Outputs[I].insert(I);
// becomes
Outputs[I].insert(In.begin(), In.end());
```
```cpp id="q3tf61"
int In[] = {1, 2, 3};
for (int I : In)
Out.insert(I);
// becomes
Out.insert(In.begin(), In.end());
```
The negative vector test is also not valid C++, so it does not provide useful
coverage.
I would narrow the first version of the check substantially, add tests for the
unsupported cases, and only then expand the matcher. For clang-tidy, a
reasonably incomplete check is acceptable. A fix-it that changes semantics or
emits uncompilable code is not.
https://github.com/llvm/llvm-project/pull/226742
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
