https://github.com/unterumarmung requested changes to this pull request.
Thanks for working on this, but I request changes for now. Please disclose the AI assistance in the PR description in line with LLVM's AI tool policy. The commits list Claude as a co-author, but the description doesn't mention it. The documentation could also be shorter and closer to the style of other clang-tidy check documentation. I think this check is useful, but splitting it into smaller PRs would make it easier to review. For example, one PR could cover the `assign` rewrites, with follow-up PRs for the other forms, or the other way around. Also please find possible opportunities to reuse functionality from already public APIs in clang or clang-tidy or other checks (in this case, you need to create a separate PR that extracts it and adds unit tests, if necessary). I also used Codex to review the implementation. It reported the following unsafe fixes, which I reproduced locally. Please address them and add regression tests: 1. The fix changes evaluation order. With `i = 0`, `vs[i++] = std::vector<int>(i + 1, 0)` creates one element, but `vs[i++].assign(i + 1, 0)` creates two. Since C++17, assignment evaluates the RHS before the LHS, whereas a member call evaluates its receiver before its arguments. 2. `assign` introduces additional element-type requirements. For a copy-constructible `T` with deleted copy assignment, `v = std::vector<T>(3, value)` compiles, but the suggested `v.assign(3, value)` does not. 3. The `resize` rewrite breaks containers of immovable elements. `v = std::vector<std::mutex>(n)` compiles, but `v.clear(); v.resize(n)` does not because vector's `resize` requires MoveInsertable elements. 4. A parenthesized assignment becomes invalid syntax: `(v = std::vector<int>(3));` is rewritten to `(v.clear(); v.resize(3));`. The parent lookup skips parentheses, but the replacement range leaves them around two statements. Please also investigate the reports in https://github.com/clang-tidy-infra/CTIT/issues/209, confirm which are false positives, and fix those with regression tests. https://github.com/llvm/llvm-project/pull/222159 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
