ojhunt wrote:

> > This PR adds a completeness check in `Sema::ActOnCXXDelete` so type-aware 
> > operator delete lookup is skipped when the pointee type is incomplete.
> > Added a test covering C++17/23 (warn) and C++26 (error).
> > Fixes #212675
> 
> Hey, just curious in case am missing anything why not just check the language 
> opts (`LangOpts`) for the cpp standard and accordingly decide if we want to 
> allow type aware allocator lookup?

People want to be able to use the feature in prior releases, and gating the 
entire feature on a specific release when this problem only occurs in an error 
path (delete IncompleteType is always an error, even if it only became that 
officially in c++23).

The question is solely how to deal with an erroneous `delete IncompleteType` 
older versions of the standard don't diagnose this error, but for TA allocation 
operators the type definition matters.

That said we should be succeeding in the type_identity instantiation which 
_should_ be sufficient to identify the existence of any potentially type aware 
candidates without failure, and alter diagnostics based on that.

The exact scenario is something like:

```cpp
namespace some_lib {
struct ClassOne {
  void* operator new(...) { return my_allocator_alloc(...); }
  void operator delete(...) { my_allocator_free(...) };
};
struct ClassTwo { 
// exact copy of ClassOne's allocators
};
struct ClassThree { ... }
}
```

```cpp
#include <some_lib.h>

struct S;
void f(S *s) {
  delete s; // this code is wrong regardless of standard version
}
```
later some_lib removes the boiler plate

```cpp
namespace some_lib {
 struct ClassOne {
  // no allocator boiler plate
};
struct ClassTwo { 
  // no allocator boiler plate
};
struct ClassThree { 
  // no allocator boiler plate }
};
void in_somelib_namespace(auto&&);

}
template <typename T>
concept InTargetNamespace = requires(T&& t) { 
in_somelib_namespace(std::forward<T>(t)); }

template <class T>
    requires InTargetNamespace<T> void *operator new(std::type_identity<T>, 
size_t, align_val_t);
template <class T>
    requires InTargetNamespace<T> void operator delete(std::type_identity<T>, 
void*, size_t, align_val_t);
```
Now the original code
#include <some_lib.h>

struct S;
void f(S *s) {
  delete s; // this code is wrong regardless of standard version
                  // but doesn't hit some_lib because S is not in some_lib's 
namespace
}
//but if we then add
void f(/*using just a forward decl header*/ some_lib::ClassOne *obj) {
  delete obj;
}
```
should be an error

the problem is that currently this is going wrong and we're only getting an 
incomplete type_identity I would guess. So the question is how to handle this 
case: an error (fragile), or a warning and decay to non-type aware, which would 
be consistent with the pre-standardized error model in which the wrong operator 
delete is called.

I realize another fragile option would be to have the use of type aware 
allocators immediately force delete to only operate on complete types in any 
part of the code base.


https://github.com/llvm/llvm-project/pull/213455
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to