This is an archive of the discontinued LLVM Phabricator instance.

Fix PR 33189: Clang assertion on template destructor declaration
ClosedPublic

Authored by kuang_he on Jun 2 2017, 8:01 AM.

Details

Summary

This patch aims to fix the bug reported at https://bugs.llvm.org/show_bug.cgi?id=33189. Clang hits an assertion when a template destructor declaration is present. This is caused by later processing that does not expect to encounter a template when looking at a destructor. The resolution is to treat the destructor as being not declared when later processing is interested in the properties of the destructor of a class.

Diff Detail

Event Timeline

kuang_he created this revision.Jun 2 2017, 8:01 AM
hubert.reinterpretcast retitled this revision from Fix Clang assertion on template destructor declaration to Fix PR 33189: Clang assertion on template destructor declaration.Jun 2 2017, 4:21 PM
hubert.reinterpretcast edited the summary of this revision. (Show Details)
hubert.reinterpretcast edited subscribers, added: cfe-commits; removed: rcraik, hubert.reinterpretcast.
lib/AST/DeclCXX.cpp
1420

As it is,

return R.empty() ? nullptr : dyn_cast<CXXDestructorDecl>(R.front());

would probably be much more succinct.

kuang_he updated this revision to Diff 101601.Jun 6 2017, 12:24 PM

Patch updated addressing comment.

Can we have this patch reviewed?

Can we get this patch reviewed by any chance?

Can we get this patch reviewed by any chance?

@kuang_he; it is customary to "ping". In this case, "Ping 2".

lib/AST/DeclCXX.cpp
1424

I think what is here is probably what we want to do. My understanding is that the code is certainly invalid before we hit this, and the case is so odd that pursuing better recovery is not worthwhile.

Do we know if we can recover from getting a FunctionTemplateDecl by some other means? Perhaps by using the result of getTemplatedDecl()? I suspect there may be problems with cases where the noexcept-specifier is dependent.

rsmith accepted this revision.Jun 23 2017, 3:46 PM
rsmith added a subscriber: rsmith.
rsmith added inline comments.
test/SemaCXX/PR33189.cpp
2–8

Please fold this into an existing test file, perhaps test/SemaTemplate/destructor-template.cpp?

This revision is now accepted and ready to land.Jun 23 2017, 3:46 PM
kuang_he updated this revision to Diff 104018.Jun 26 2017, 2:00 PM
hubert.reinterpretcast edited the summary of this revision. (Show Details)Jun 30 2017, 3:33 PM
rsmith accepted this revision.Jun 30 2017, 3:37 PM

Thanks, do you need someone to commit this for you?