[Lldb-commits] [PATCH] D75761: Fix to get the AST we generate for function templates to be closer to what clang generates and expects

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It's a pity that the clang's DW_AT_name value is so ambiguous. For example, gcc would output the name in the commit message as `operator< <A::B>`, which is a lot more parsable (for computers and people). Have you looked at changing clang's output to make it more like gcc's ? I know it would only help new compilers, but maybe for such a special case, this does not matter?

Or, even if we do end up adding some compat parsing code, that would reduce the need for it to be super exact. I'm pretty sure that the current code does not handle all cases correctly. E.g., I believe it will break on something like `operator<<<&operator>> >` (mangled name `_ZlsIXadL_Zrs1AS0_EEEvS0_S0_`, godbolt link <https://godbolt.org/z/-c68Dz>).

As an alternative, we could try extracting the same information from the mangled name (via llvm::ItaniumPartialDemangler::getFunctionBaseName), at least when DW_AT_linkage_name is present (not all compilers emit that attribute but I am not sure if anyone has tested if lldb actually works without it).



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:839
+  size_t RightAngleCount = Name.count('>');
+  size_t LeftAngleCount = Name.count('<');
+
----------------
aprantl wrote:
> We are scanning the entire string multiple times here and that seems unnecessarily expensive since C++ template function names get looong. I think we could do this as a single for-loop with a state machine instead, without being too difficult to read?
Would it be sufficient to always strip one `<` from the first sequence of `<`'s that you find? You can't have two template `<` one after the other, so in a valid name, the last `<` will be the template angle bracket, while everything else must be a part of the operator name.

So, something like `return {Name.data(), Name.find_first_not_of('<', Name.find_first_of('<')) -1}` (with some error checks and an explicit check for the spaceship operator).


================
Comment at: lldb/test/API/lang/cpp/template-function/main.cpp:1
 template<typename T>
 int foo(T t1) {
----------------
Do we have a test for the spaceship operator?


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

https://reviews.llvm.org/D75761