[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