1406197 - [-Wsized-deallocation] warning in mozalloc.h

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Tom Ritter [:tjr]]

gcc 6.4.0 in the mingw build is throwing these warnings:


>   /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:228:6: warning: the program should also define 'void operator delete(void*, std::size_t)' [-Wsized-deallocation]
>    void operator delete(void* ptr) MOZALLOC_THROW_IF_HAS_EXCEPTIONS
>         ^~~~~~~~
>   /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:240:6: warning: the program should also define 'void operator delete [](void*, std::size_t)' [-Wsized-deallocation]
>    void operator delete[](void* ptr) MOZALLOC_THROW_IF_HAS_EXCEPTIONS
>         ^~~~~~~~

From what I've been able to read[0][1], I think I can resolve this by adding functions that just call `return free_impl(ptr);` and ignore the size argument, which I will do unless anyone thinks that's a bad approach.

[0] https://isocpp.org/files/papers/n3778.html
[1] https://github.com/cpputest/cpputest/pull/987

Comment 1

[Tom Ritter [:tjr]]

Attachment: Bug 1406197 Declare sized deallocators (that ignore the size) to correct a MinGW warning

Review commit: https://reviewboard.mozilla.org/r/187320/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/187320/

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8916069 [details]
Bug 1406197 Declare sized deallocators (that ignore the size) to correct a MinGW warning

https://reviewboard.mozilla.org/r/187320/#review192374

::: memory/mozalloc/mozalloc.h:234
(Diff revision 1)
>  {
>      return free_impl(ptr);
>  }
>  
>  MOZALLOC_EXPORT_NEW MOZALLOC_INLINE
> +void operator delete(void* ptr, size_t size) MOZALLOC_THROW_IF_HAS_EXCEPTIONS

We should probably declare these only when we're compiling with C++14.

::: memory/mozalloc/mozalloc.h:252
(Diff revision 1)
>  {
>      return free_impl(ptr);
>  }
>  
>  MOZALLOC_EXPORT_NEW MOZALLOC_INLINE
> +void operator delete[](void* ptr, size_t size) MOZALLOC_THROW_IF_HAS_EXCEPTIONS

Likewise here?

Comment 3

[Tom Ritter [:tjr]]

Comment on attachment 8916069 [details]
Bug 1406197 Declare sized deallocators (that ignore the size) to correct a MinGW warning

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/187320/diff/1-2/

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8916069 [details]
Bug 1406197 Declare sized deallocators (that ignore the size) to correct a MinGW warning

https://reviewboard.mozilla.org/r/187320/#review193056

::: memory/mozalloc/mozalloc.h:233
(Diff revision 2)
>  void operator delete(void* ptr) MOZALLOC_THROW_IF_HAS_EXCEPTIONS
>  {
>      return free_impl(ptr);
>  }
>  
> +#if __cplusplus >= 201300L

why not 201402L?

Comment 5

[Tom Ritter [:tjr]]

Comment on attachment 8916069 [details]
Bug 1406197 Declare sized deallocators (that ignore the size) to correct a MinGW warning

https://reviewboard.mozilla.org/r/187320/#review193244

::: memory/mozalloc/mozalloc.h:233
(Diff revision 2)
>  void operator delete(void* ptr) MOZALLOC_THROW_IF_HAS_EXCEPTIONS
>  {
>      return free_impl(ptr);
>  }
>  
> +#if __cplusplus >= 201300L

You're right. 

I was following https://bugzilla.mozilla.org/show_bug.cgi?id=1325632#c29 and https://bugzilla.mozilla.org/show_bug.cgi?id=1392643 which account for gcc 4.9 having Draft C++14 support, and used that constan, but this warning didn't show up until gcc 6 which has full support.

Comment 6

[Tom Ritter [:tjr]]

Comment on attachment 8916069 [details]
Bug 1406197 Declare sized deallocators (that ignore the size) to correct a MinGW warning

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/187320/diff/2-3/

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8916069 [details]
Bug 1406197 Declare sized deallocators (that ignore the size) to correct a MinGW warning

https://reviewboard.mozilla.org/r/187320/#review193452

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d346efc8449b
Declare sized deallocators (that ignore the size) to correct a MinGW warning r=glandium

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/d346efc8449b