[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type it has incomplete type debug info.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:681
+        auto &ts = llvm::cast<TypeSystemClang>(*ct.GetTypeSystem());
+        ts.GetMetadata(&tag)->SetIsForcefullyCompleted();
+      }
----------------
zequanwu wrote:
> zequanwu wrote:
> > labath wrote:
> > > zequanwu wrote:
> > > > rnk wrote:
> > > > > Is this what we do for DWARF? The same kind of situation seems like it can arise, where clang requires a type to be complete, but the information is missing, and we still try to proceed with evaluation.
> > > > I think it's here: https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L221-L250, relevant: https://reviews.llvm.org/D85968.
> > > The trick is to do the forced completion only when the type is used within contexts where the c++ rules require it to be complete. something like `class A; A* a;` is perfectly legal c++. `class A; class B : A {};` is not. You can't do this from within the completion callback. In DWARF code, we check for this when we're parsing the enclosing entity (so, when we're parsing `B`, we'd check, and if needed, "forcefully complete" the class `A`.
> > What does dwarf plugin do in this case that we don't have debug info for class A: `class A; class B : A {};`? The problem I'm trying to fix is when the base class has no debug info to complete it.
> @labath, Do you see anything would go wrong if we forcefully complete a class that has no debug info? It seems fine, for the chrome crash report I was checking.
I don't think anything will blow up, if that's what you're asking. The way I think this will manifest itself is that various opaque types that are supposed to be incomplete (e.g. this is often the case with the `FILE` type in the C library) will suddenly become complete (and empty). That in turn can have various (more or less subtle) effects on the way these objects can be viewed and manipulated by the user. It's not the end of the world, but given that this forceful completion business is essentially a giant hack, I think it makes sense to limit its scope as much as possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066