[Lldb-commits] [PATCH] D56126: [NativePDB] Add basic support of methods recostruction in AST

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov marked an inline comment as done.
aleksandr.urakov added a comment.

In D56126#1342114 <https://reviews.llvm.org/D56126#1342114>, @zturner wrote:

> I think it's probably best to skip this part for now and come back to it later.  The only thing that will be missing is the ability to use member function templates in expressions.  Of course we need this eventually, but probably there is more useful stuff to work on first.


Yes, I agree. So I can continue with virtual bases support (but only since the middle of January), good?

> One idea for implementing this though might be to add a pre-processing step of the publics stream similar to how we pre-process the TPI stream.  This would also allow us to find mangled names of global functions as well (currently even for global functions, we create an empty `Mangled` structure when LLDB asks us for the mangled name).  One pass over the publics stream should be simpler and more straightforward than a pass over every single module's symbol stream.  From there we have the address, which tells us the module, and then we can use the `CompilandIndexItem::FindSymbolsByVa()` to find the `S_GPROC32` record, and from there find the type.  But I think this is a large effort for low value, so we should maybe wait until the easier stuff is done first.  I haven't looked at the rest of the patch yet, but I will in a little bit.  Thanks!

Thanks for comment! I think that such preprocessing is a good idea. But is it guaranteed that we will have a record in the publics stream for any function?



================
Comment at: lit/SymbolFile/NativePDB/ast-methods.cpp:33
+// CHECK: | |-CXXMethodDecl {{.*}} overloaded_method 'int (char) __attribute__((thiscall))'
+// CHECK: | | `-ParmVarDecl {{.*}} 'char'
+// CHECK: | `-CXXMethodDecl {{.*}} overloaded_method 'int (char, int, ...)'
----------------
The one thing I've forgot to say about is that parameter names are in `Symbols` stream too. Is it ok for now to proceed without them too?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56126