This is an archive of the discontinued LLVM Phabricator instance.

More correct handling of error cases C++ name parser
ClosedPublic

Authored by eugene on Feb 5 2018, 4:49 PM.

Details

Summary

Now incorrect type argument that looks like T<A><B> doesn't cause an assert, but just a parsing error.

Bug: 36224

Diff Detail

Repository
rL LLVM

Event Timeline

eugene updated this revision to Diff 132916.Feb 5 2018, 4:49 PM
eugene created this revision.

fix formating

labath accepted this revision.Feb 6 2018, 1:57 AM

Being more resilient when handling demangler outputs seems like a good thing, but I think it is important to understand what made the demangler produce that output in the first place, to make sure we aren't missing anything.

Also, I think I found one more possible issue with this code.

source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
199 ↗(On Diff #132916)

While looking at the bug, this part here struck me as dubious.

Can you check that this properly handles a name like F<(3)>>(1)> f<3, 1>() (which is the demangled form of _Z1fILi3ELi1EE1FIXrsT_T0_EEv).

This revision is now accepted and ready to land.Feb 6 2018, 1:57 AM
eugene added inline comments.Feb 6 2018, 11:05 AM
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
199 ↗(On Diff #132916)

oh my, you're right. this code doesn't account for shifts in the name. I'm going to submit this code anyway and then think about it.

This revision was automatically updated to reflect the committed changes.