labath added inline comments.

================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1
+#include "/tmp/debug.h"
 #include "PdbAstBuilder.h"
----------------
oops?


================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:96
   CompilerType method_ct = m_ast_builder.ToCompilerType(method_qt);
-  lldb::opaque_compiler_type_t derived_opaque_ty = 
m_derived_ct.GetOpaqueQualType();
+  PdbAstBuilder::RequireCompleteType(method_ct);
+  lldb::opaque_compiler_type_t derived_opaque_ty =
----------------
I suppose this won't hurt, but `RequireCompleteType` will not actually do 
anything for method types, right ?

Regarding method types, there is a slightly different problem. C++ rules 
require that in code like
`struct B1 { virtual A1* f(); }; struct B2:B1 { A2* f(); };`, the types `A1` 
and `A2` must be complete (because that code is only valid if A1 is a base of 
A2). However, I think that's better left to a separate patch.


================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:159
   CompilerType member_ct = m_ast_builder.ToCompilerType(member_type);
+  PdbAstBuilder::RequireCompleteType(member_ct);
 
----------------
Are you sure about static member part? Something like `struct A; struct B { 
static A a; };` will compile fine (unlike `struct A; struct B { A a; };`), so 
I'd hope that the expression evaluator could handle an incomplete static 
member. It would be better if `p B::a` produces something like `error: variable 
has incomplete type 'A'` instead of printing an empty struct.


================
Comment at: lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp:25
+B b;
+B b_array[3] = {};
+A C::static_a = A();
----------------
The `A a_array[]` case is more interesting, because here the class might be 
(and probably will be) completed as a part of completing B. It would also be 
better to use a different incomplete class for each test case, as a class can 
be completed only once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to