================
@@ -279,7 +281,7 @@ void UseAfterMoveFinder::getDeclRefs(
if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) {
// Ignore uses of a standard smart pointer that don't dereference the
// pointer.
- if (Operator || !isStandardSmartPointer(DeclRef->getDecl())) {
+ if (Operator || !isStandardResettableOwner(DeclRef->getDecl())) {
----------------
5chmidti wrote:
> `std::optional::has_value` seems OK (it's no different than `operator==(const
> std::unique_ptr &, std::nullptr_t)`, both of which are OK as they don't
> actually access the moved-from value)+
[...]
I'm kind of torn here. I'd much rather prioritize C++17 over C++23, since I
expect `reset` to be used much more, but the monadic operators might eventually
become commonplace too.
Actually, it is for `optional` and `any` (though `any` is left in a fully
unspecified state, unlike `optional`). It's not just the monadic operators, but
e.g., `has_value` together with `value` (etc.): https://godbolt.org/z/hc4MEPWj8
```c++
#include <iostream>
#include <optional>
#include <vector>
void sink(std::optional<std::vector<int>> b) {
if (b.has_value()) {
std::cout << "moved to: " << b.value().front() << '\n' << std::flush;
}
}
int main() {
auto a = std::optional<std::vector<int>>{{42}};
sink(std::move(a));
if (a.has_value()) { // moved-from optional `a` still returns true here
// accessing moved from contained object
std::cout << "moved from: " << a.value().front() << '\n' << std::flush;
}
}
```
Both post-move accesses would not be considered as accessing the contained
value due to the focus on the dereferencing operators of smart pointers.
> But then again, it seems(?) the fact that we don't warn on `operator==` is
> just an accident caused by it being a non-member, not intentional?
All operations except `operator*`, `operator->` or `operator[]` are completely
fine from what I can tell. The ptr of smart pointers is set to `nullptr`, which
allows for things like `operator==` or `operator bool` to be valid uses,
because unlike `optional` they don't return `true` when the internal object has
been moved from/is a `nullptr` (`weak_ptr` is different, but has similar
semantics to the other smart pointers).
> I guess, in theory, what we need to do here is to distinguish the different
> methods [...]
> Barring that, a blanket rule should either allow ~all members, or prohibit
> ~all members.
For `optional` and `any` I'd say we swap the logic around and only allow
`reset`, `emplace` and assignments (already exists) as resetting the state, and
warn on anything else, while the smart_ptr case only disallows the operators it
does right now.
> but I don't really have the bandwidth to try to figure out how to implement
> that...
I would've said that this PR should be blocked until the above is handled
correctly, but actually, the PR as is, is removing some false-positives by
considering the calls to `reset` as resetting the state. So there are no real
negatives attached to this PR, that weren't there before, it's only improving
the state. The changes outlined above make some negatives positives, which
could be done in a follow-up (+issue in case you do not end up implementing it)
(in one go would be nicer, of course).
https://github.com/llvm/llvm-project/pull/114255
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
