[PATCH] D111419: [Demangle] Add support for D types back referencing

[Luís Ferreira via Phabricator via llvm-commits]

ljmf00 added a comment.

I also added another test to include invalid back reference position with `_D8demangle3fooQXXXx`.



================
Comment at: llvm/unittests/Demangle/DLangDemangleTest.cpp:44
+      {"_D8demangle3ABCQeQaaaa1ai", nullptr},
+      {"_D8demangle4ABCiQfQh1aQh", "demangle.ABCi.ABCi.ABCi.a"},
+      {"_D8demangle4ABCiQfQh1aQaaa", nullptr}
----------------
dblaikie wrote:
> ljmf00 wrote:
> > dblaikie wrote:
> > > /might/ be easier to follow if each case is tested in a separate mangled name? (helps to see which things are varying, which things are the same, so the intention of each test is clearer?)
> > > 
> > > Hmm, yeah, I'm not sure I follow what this is testing. It's got several backrefs, perhaps where one would suffice? (similarly with the previous ABC.ABC.ABC.a test, for what it's worth - not quite following that either) & they look like they'd all parse through parseQualified... 8demangle, 4ABCi, Qf, Qh, 1a, Qh - looks all like that would be resolved by parseIdentifier (calling the base case of parseLName for the numerically prefixed ones, and the parseSymbolBackref for the Q ones) called in parseQualified, 
> > Well, I added an overcomplicated test. I simplified it to use only one back reference, since referencing backreferences are not in the specification. The idea behind this backreference concept is to calculate the position offset to the current position but backwards, by converting the difference of a char ASCII value with 'a' or 'A' to the desired shifted position and relocating the pointer with that value, so that Ret points to that reference. For the current test case example `_D8demangle4ABCi1aQd` `Qd` is a backreference for position 4, counting from `d` char, so decoding it points to `i`. Since `i` is a valid type sequence, it succeeded.
> > 
> > I added an invalid test case with a comment explaining it. `LastBackRef` is currently being used to check if the `LastBackRef` is the same as the currently decoded backreference, and therefore detecting a recursive backreference. This needs to be checked since a type can technically point to a letter. This logic can be later extended when pointing to non-recursive backreferences is a thing (I'm currently discussing this and doing some test cases to implement this in the official spec and the reference compiler) otherwise, checking for the 'Q' letter would be sufficient.
> Might be handy to include some of the explanation of the `_D8demangle4ABCi1aQd` from this discussion in a comment in the test 
Added the suggested explanation on both patches. Updated the previous one as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111419/new/

https://reviews.llvm.org/D111419