unterumarmung wrote:
I was wrong to argue that the constructor and `reset` matchers should behave
identically. The constructor matcher requires the allocated type to match the
pointee type, so the derived-to-base case that causes this bug doesn't apply
there. Sorry for the confusion.
We, Codex and I, tested both matcher paths locally. There are two remaining
false positives and one valid modernization suppressed by this patch.
The destructor guard misses class-template specializations and aliases:
```cpp
#include <memory>
struct Base {};
template<class T>
struct PrivateChild : Base {
private:
~PrivateChild();
};
struct DeletedChild : Base {
~DeletedChild() = delete;
};
using Alias = DeletedChild;
void test() {
std::unique_ptr<Base> p;
p.reset(new PrivateChild<int>());
p.reset(new Alias());
}
```
Both original expressions compile. The check replaces them with:
```cpp
p = std::make_unique<PrivateChild<int>>();
p = std::make_unique<Alias>();
```
Both replacements fail to compile because of the private or deleted destructor.
Could we desugar the pointee before matching `recordType` and add regression
tests for these cases?
For the constructor matcher, the new guard suppresses a valid replacement:
```cpp
#include <memory>
struct Child {
friend struct std::default_delete<Child>;
private:
~Child() = default;
};
void test() {
std::unique_ptr<Child> p(new Child());
}
```
Both this declaration and `auto p = std::make_unique<Child>();` compile. The
check suggested that modernization before this patch, but now suppresses it.
Could we keep the new destructor guard on the `reset` matcher only?
https://github.com/llvm/llvm-project/pull/229802
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
