Page MenuHomePhabricator

Propagate missing empty exception spec from function declared in system header
Needs ReviewPublic

Authored by d.zobnin.bugzilla on Mar 31 2016, 10:32 AM.

Details

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.

Diff Detail

Event Timeline

d.zobnin.bugzilla retitled this revision from to Propagate missing empty exception spec from function declared in system header.
d.zobnin.bugzilla updated this object.
d.zobnin.bugzilla added a subscriber: cfe-commits.

Friendly ping, please take a look.

Thank you,
Denis Zobnin

rjmccall edited edge metadata.Apr 11 2016, 8:42 PM

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 edited edge metadata.

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

Friendly ping, please take a look.

Thank you,
Denis Zobnin

rjmccall added inline comments.Apr 22 2016, 11:28 AM
lib/Sema/SemaExceptionSpec.cpp
260

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

rsmith added a subscriber: rsmith.Apr 22 2016, 12:30 PM
rsmith added inline comments.
lib/Sema/SemaExceptionSpec.cpp
260

getFirstDecl is cheap these days.

rjmccall added inline comments.Apr 22 2016, 1:18 PM
lib/Sema/SemaExceptionSpec.cpp
260

Ah, nevermind then.