Define `void operator delete[](void *p, std::size_t n)` in new_delete.cpp #430

Merged

jonathangjertsen merged 2 commits from add-missing-delete-signature into develop

2021-05-24 16:47:30 -05:00

jonathangjertsen commented

2021-05-21 10:40:23 -05:00

(Migrated from github.com)

This prevents the following warning when compiling with -Wextra:

[redacted]/pico-sdk/src/rp2_common/pico_standard_link/new_delete.cpp:24:6: error: the program should also define 'void operator delete [](void*, unsigned int)' [-Werror=sized-deallocation]
   24 | void operator delete[](void *p) noexcept { std::free(p); }
      |      ^~~~~~~~

This function signature was introduced in C++14. I don't think that should cause any issues, but I'm not sure.

This prevents the following warning when compiling with `-Wextra`: ```C [redacted]/pico-sdk/src/rp2_common/pico_standard_link/new_delete.cpp:24:6: error: the program should also define 'void operator delete [](void*, unsigned int)' [-Werror=sized-deallocation] 24 | void operator delete[](void *p) noexcept { std::free(p); } | ^~~~~~~~ ``` This function signature was introduced in C++14. I don't _think_ that should cause any issues, but I'm not sure.

Reneg973 commented

2021-05-22 02:47:28 -05:00

(Migrated from github.com)

see https://github.com/raspberrypi/pico-sdk/issues/304

fivdi (Migrated from github.com) reviewed

2021-05-22 04:43:54 -05:00

src/rp2_common/pico_standard_link/new_delete.cpp

The C++ new header only adds the declarations for the sized deallocation functions if __cpp_sized_deallocation is true.

So, should the sized deallocation function that is being added with this PR (and the already existing size deallocation function on line 20) be conditionally compiled using __cpp_sized_deallocation (see here)?

Here is what the new header has:

#if __cpp_sized_deallocation
void operator delete(void*, std::size_t) _GLIBCXX_USE_NOEXCEPT
  __attribute__((__externally_visible__));
void operator delete[](void*, std::size_t) _GLIBCXX_USE_NOEXCEPT
  __attribute__((__externally_visible__));
#endif

The C++ new header only adds the declarations for the sized deallocation functions if `__cpp_sized_deallocation` is true. So, should the sized deallocation function that is being added with this PR (and the already existing size deallocation function on [line 20](https://github.com/raspberrypi/pico-sdk/blob/5446fa7ae922dd1aced35028799c4f615fd0dad9/src/rp2_common/pico_standard_link/new_delete.cpp#L20)) be conditionally compiled using `__cpp_sized_deallocation` (see [here](https://en.cppreference.com/w/cpp/feature_test))? Here is what the new header has: ```c++ #if __cpp_sized_deallocation void operator delete(void*, std::size_t) _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__)); void operator delete[](void*, std::size_t) _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__)); #endif ```

jonathangjertsen (Migrated from github.com) reviewed

2021-05-22 14:12:17 -05:00

src/rp2_common/pico_standard_link/new_delete.cpp

I think it will work either way. I looked up some other implementations of this fix and found one case where the conditional is included (here), and two where it was not (here and here).

I think it will work either way. I looked up some other implementations of this fix and found one case where the conditional is included ([here](https://hg.mozilla.org/mozilla-central/rev/d346efc8449b)), and two where it was not ([here](https://hg.openjdk.java.net/jdk/jdk/rev/4ec9fc2b2f0d) and [here](https://github.com/cpputest/cpputest/issues/957)).