[libcxx-commits] [PATCH] D120633: [libc++] Better handling for zero-sized types
Arthur O'Dwyer via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Feb 27 10:27:22 PST 2022
Quuxplusone added a comment.
In D120633#3347916 <https://reviews.llvm.org/D120633#3347916>, @philnik wrote:
> The tests should be in `libcxx/`, since it's an extension, right?
I'd say no, because it's a compiler extension (in multiple compilers), not a libc++ extension. I do expect the new tests to need `UNSUPPORTED: msvc`, but decided I could let buildkite tell me that instead of guessing ahead.
> Is there a reason you only check with `make_unique`? If you don't use `make_unique` the `UNSUPPORTED: c++11` could be removed from the test.
`make_unique` has its own overload set that we want to make sure works as expected. However, I suppose I should also check the behavior of `std::default_delete<A>` directly, in a new test, and that one can be enabled in C++11. Will fix.
================
Comment at: libcxx/include/__memory/unique_ptr.h:50-53
- static_assert(sizeof(_Tp) > 0,
- "default_delete can not delete incomplete type");
- static_assert(!is_void<_Tp>::value,
- "default_delete can not delete incomplete type");
----------------
philnik wrote:
> Could you check the compiler diagnostics before and after?
Hmm, I guess the diagnostic for deleting an incomplete or void pointer is somehow just a warning! I wonder what the compiler thinks it means. (How could it possibly know how much memory to free, without a size?)
So I suppose it would be reasonable to keep the assert on lines 50-51, and just change its condition to `sizeof(_Tp) >= 0`. That'll still be false for `void`, so we still don't need the assert on lines 52-53.
Code:
```
#include <memory>
struct I;
void f(std::default_delete<I> d, I *p) { d(p); }
void f(std::default_delete<void> d, void *p) { d(p); }
void f(I *p) { delete p; }
void f(void *p) { delete p; }
```
Before (i.e. in `main`):
```
x.cpp:7:16: warning: deleting pointer to incomplete type 'I' may cause undefined behavior [-Wdelete-incomplete]
void f(I *p) { delete p; }
^ ~
x.cpp:3:8: note: forward declaration of 'I'
struct I;
^
x.cpp:8:19: warning: cannot delete expression with pointer-to-'void' type 'void *' [-Wdelete-incomplete]
void f(void *p) { delete p; }
^ ~
In file included from x.cpp:1:
In file included from /Users/aodwyer/llvm-project/build2/bin/../include/c++/v1/memory:820:
In file included from /Users/aodwyer/llvm-project/build2/bin/../include/c++/v1/__memory/shared_ptr.h:24:
/Users/aodwyer/llvm-project/build2/bin/../include/c++/v1/__memory/unique_ptr.h:50:19: error: invalid application of 'sizeof' to an incomplete type 'I'
static_assert(sizeof(_Tp) > 0,
^~~~~~~~~~~
x.cpp:4:43: note: in instantiation of member function 'std::default_delete<I>::operator()' requested here
void f(std::default_delete<I> d, I *p) { d(p); }
^
x.cpp:3:8: note: forward declaration of 'I'
struct I;
^
In file included from x.cpp:1:
In file included from /Users/aodwyer/llvm-project/build2/bin/../include/c++/v1/memory:820:
In file included from /Users/aodwyer/llvm-project/build2/bin/../include/c++/v1/__memory/shared_ptr.h:24:
/Users/aodwyer/llvm-project/build2/bin/../include/c++/v1/__memory/unique_ptr.h:50:19: error: invalid application of 'sizeof' to an incomplete type 'void'
static_assert(sizeof(_Tp) > 0,
^~~~~~~~~~~
x.cpp:5:49: note: in instantiation of member function 'std::default_delete<void>::operator()' requested here
void f(std::default_delete<void> d, void *p) { d(p); }
^
```
After (i.e. with the current revision of this PR):
```
x.cpp:7:16: warning: deleting pointer to incomplete type 'I' may cause undefined behavior [-Wdelete-incomplete]
void f(I *p) { delete p; }
^ ~
x.cpp:3:8: note: forward declaration of 'I'
struct I;
^
x.cpp:8:19: warning: cannot delete expression with pointer-to-'void' type 'void *' [-Wdelete-incomplete]
void f(void *p) { delete p; }
^ ~
2 warnings generated.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120633/new/
https://reviews.llvm.org/D120633
More information about the libcxx-commits
mailing list