Propagate missing empty exception spec from function declared in system header

[• d.zobnin.bugzilla]

Summary

If new function is missing empty exception specification "throw()" and "throw()" came from system header, don't emit errors. It is already implemented in Clang, but taking into account only last redeclaration of a function. Look for declaration in system header up to the first declaration in redeclaration chain, as GCC seems to do.

This patch enables compilation of Facebook/ds2 application.

[• d.zobnin.bugzilla]

Friendly ping, please take a look.

Thank you,
Denis Zobnin

[rjmccall]

IIRC, isInSystemHeader isn't a terribly cheap check. Please extract this into a function that only needs to be called if the other checks succeed.

You should also be able to just ask about the first declaration. There are contrived situations where isInSystemHeader can be false for the first declaration and true for later ones, but we don't need to fall over ourselves supporting them.

[• d.zobnin.bugzilla]

John, thanks for the review!

Please take a look at the updated patch -- did I understand your comments correctly?
The test passes and ds2 application is compiled successfully.

Thank you,
Denis Zobnin

[• d.zobnin.bugzilla]

Friendly ping, please take a look.

Thank you,
Denis Zobnin

[rjmccall]

lib/Sema/SemaExceptionSpec.cpp

260

Please find a way to sink the call to getFirstDecl() to the point where we need it.

[rsmith]

lib/Sema/SemaExceptionSpec.cpp

260

getFirstDecl is cheap these days.

[rjmccall]

lib/Sema/SemaExceptionSpec.cpp

260

Ah, nevermind then.

lib/Sema/SemaExceptionSpec.cpp

bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {
   // The new function declaration is only missing an empty exception
   // specification "throw()". If the throw() specification came from a
   // function in a system header that has C linkage, just add an empty
   // exception specification to the "new" declaration. This is an
   // egregious workaround for glibc, which adds throw() specifications
   // to many libc functions as an optimization. Unfortunately, that
   // optimization isn't permitted by the C++ standard, so we're forced
   // to work around it here.
-  if (MissingEmptyExceptionSpecification && NewProto &&
-      (Old->getLocation().isInvalid() ||
-       Context.getSourceManager().isInSystemHeader(Old->getLocation())) &&
-      Old->isExternC()) {
+  FunctionDecl *First = Old->getFirstDecl();
+  if (MissingEmptyExceptionSpecification && NewProto && First->isExternC() &&
+      (First->getLocation().isInvalid() ||
+       Context.getSourceManager().isInSystemHeader(First->getLocation()))) {
     New->setType(Context.getFunctionType(
         NewProto->getReturnType(), NewProto->getParamTypes(),
         NewProto->getExtProtoInfo().withExceptionSpec(EST_DynamicNone)));
     return false;
   }
 
   const FunctionProtoType *OldProto =
     Old->getType()->castAs<FunctionProtoType>();

test/SemaCXX/Inputs/malloc.h

 extern "C" {
 extern void *malloc (__SIZE_TYPE__ __size) throw () __attribute__ ((__malloc__)) ;
+
+void MyFunc() throw();
 }

test/SemaCXX/builtin-exception-spec.cpp

 // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only -verify %s
 // expected-no-diagnostics
 #include <malloc.h>
 
 extern "C" {
 void *malloc(__SIZE_TYPE__);
+
+void MyFunc() __attribute__((__weak__));
+void MyFunc() { return; }
 }