[patch] eliminate noisy warning on MSVC

The attached patch eliminates a noisy warning building Clang with Visual
C++.

Essentially, if MSVC sees an operator new overload, it expects to see the
matching operator delete in case an exception is thrown and it needs to
reclaim the storage. In this case the warning is redundant, as the operator
new overload is declared with an empty throw specification.

I still think it worth taking the patch as:

i/ one forward declaration eliminates around 5500 warnings, totally swamping
signal-to-noise in MSVC.
ii/ it is generally good practice to provide such overloads in pairs, and
I'm more comfortable following the general rule than grokking the code to
see why this is a special case - AFAICT there is no benefit/optimization
from not supplying the overload.

It is also an easy way for me to see how the patch submission process works
;¬)

BTW, among other things I am working on a doc patch for "building Clang on
Windows" which might make MSVC warnings more of an issue (if successful!)

AlisdairM

MSVCWarning.patch (756 Bytes)

I believe you'll need a definition. This helloworld:

#include <new>
#include <cstdlib>
#include <string>

class A {};

void* operator new(std::size_t s, const A&) throw()
{
     return std::malloc(s);
}

void operator delete(void*, const A&) throw();

int main()
{
     std::string* p = new (A()) std::string("some text");
}

doesn't link on g++ platforms:

Undefined symbols:
   "operator delete(void*, A const&)", referenced from:
       _main in ccqdNwOY.o
ld: symbol(s) not found
collect2: ld returned 1 exit status

The definition is not unneeded if the object being constructed can throw in its constructor. A "nothrow" new expression *can* throw, and this is when the matching placement delete is called:

#include <new>
#include <cstdlib>
#include <iostream>

class A {};

int new_called = 0;
int delete_called = 0;

void* operator new(std::size_t s, const A&) throw()
{
     ++new_called;
     return std::malloc(s);
}

void operator delete(void* p, const A&) throw()
{
     ++delete_called;
     free(p);
}

struct B
{
     B() {throw 1;}
};

int main()
{
     try
     {
         new (A()) B;
     }
     catch (...)
     {
     }
     std::cout << "new_called = " << new_called << '\n';
     std::cout << "delete_called = " << delete_called << '\n';
}

new_called = 1
delete_called = 1

-Howard

From: Howard Hinnant [mailto:hhinnant@apple.com]
Sent: 13 June 2009 20:14
To: AlisdairM (public)
Cc: cfe-dev@cs.uiuc.edu
Subject: Re: [cfe-dev] [patch] eliminate noisy warning on MSVC

The attached patch eliminates a noisy warning building Clang with
Visual C++.

Essentially, if MSVC sees an operator new overload, it expects to
see the matching operator delete in case an exception is thrown and
it needs to reclaim the storage. In this case the warning is
redundant, as the operator new overload is declared with an empty
throw specification.

I believe you'll need a definition. This helloworld:

#include <new>
#include <cstdlib>
#include <string>

class A {};

void* operator new(std::size_t s, const A&) throw() {
     return std::malloc(s);
}

void operator delete(void*, const A&) throw();

int main()
{
     std::string* p = new (A()) std::string("some text"); }

doesn't link on g++ platforms:

Undefined symbols:
   "operator delete(void*, A const&)", referenced from:
       _main in ccqdNwOY.o
ld: symbol(s) not found
collect2: ld returned 1 exit status

Note that both operator new and operator delete *are* defined, but in both
cases the definition is found in ASTContext.h.

The definition is not unneeded if the object being constructed can
throw in its constructor. A "nothrow" new expression *can* throw, and
this is when the matching placement delete is called:

#include <new>
#include <cstdlib>
#include <iostream>

class A {};

int new_called = 0;
int delete_called = 0;

void* operator new(std::size_t s, const A&) throw() {
     ++new_called;
     return std::malloc(s);
}

void operator delete(void* p, const A&) throw() {
     ++delete_called;
     free(p);
}

struct B
{
     B() {throw 1;}
};

int main()
{
     try
     {
         new (A()) B;
     }
     catch (...)
     {
     }
     std::cout << "new_called = " << new_called << '\n';
     std::cout << "delete_called = " << delete_called << '\n'; }

new_called = 1
delete_called = 1

In that case, my patch may be more than theoretical, and I should Either
update or remove the comment on the delete operator.

AlisdairM

Looks great to me, applied here:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20090608/018143.html

Note that clang (with g++) builds with -fno-exceptions, so I think this really really should be fine. I tweaked the comment in the patch to be more grammatical, please end sentences with proper punctuation etc. Thanks for the patch!

-Chris