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

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

ljmf00 added a comment.

I added comments to the test cases and fixed C like comments, ++I instead of I++ and other requested changes on previous patches. Can you please re-review it?



================
Comment at: llvm/unittests/Demangle/DLangDemangleTest.cpp:44
+      {"_D8demangle3ABCQeQaaaa1ai", nullptr},
+      {"_D8demangle4ABCiQfQh1aQh", "demangle.ABCi.ABCi.ABCi.a"},
+      {"_D8demangle4ABCiQfQh1aQaaa", nullptr}
----------------
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.


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

https://reviews.llvm.org/D111419