This is an archive of the discontinued LLVM Phabricator instance.

[MS Demangler] Properly handle function parameter list back-references.
ClosedPublic

Authored by zturner on Jul 26 2018, 2:46 PM.

Details

Summary

Properly demangle function parameter back-references.

Previously we treated lists of function parameters and template parameters the same. There are some important differences with regards to back-references, and some less important differences regarding which characters can appear before or after the name.

The important differences are that with a given type T, all instances of a function parameter list share the same global back-ref table. Specifically, if X and Y are function pointers, then there are 3 entities in the declaration X func(Y) which all affect and are affected by the master parameter back-ref table. 1) The parameter list of X's function type, 2) the parameter list of func itself, and 3) The parameter list of Y's function type.

The previous code would create a back-reference table that was local to a single parameter list, so it would not be shared across parameter lists.

This was discovered when porting ms-back-references.test from clang's mangling tests. All of these tests should now pass with the new changes.

In doing so, I split the function for parsing template and function parameters into two separate functions. This makes the template parameter list parsing code in particular very small and easy to understand now.

Diff Detail

Event Timeline

zturner created this revision.Jul 26 2018, 2:46 PM
zturner retitled this revision from [MS Demangler] to [MS Demangler] Properly handle function parameter list back-references..Jul 26 2018, 2:46 PM
rnk accepted this revision.Jul 26 2018, 3:08 PM

lgtm

llvm/lib/Demangle/MicrosoftDemangle.cpp
1595

s/memorizing/memoizing/

This revision is now accepted and ready to land.Jul 26 2018, 3:08 PM
zturner added inline comments.Jul 26 2018, 3:11 PM
llvm/lib/Demangle/MicrosoftDemangle.cpp
1595

Yea I should fix this, but it's spelled memorize throughout in many places throughout the file. So it should probably be a follow-up that fixes all places at once.

This revision was automatically updated to reflect the committed changes.