[clang] [lldb] [ASTImporter][lldb] Avoid implicit imports in VisitFieldDecl (PR #107828)

[Michael Buch via cfe-commits]

Michael137 wrote:

> As I understand it, minimal import is used in LLDB for performance reasons, so we don't waste time parsing and loading AST elements that we don't need. It sounds like a fine idea in general. Implicit imports of external sources in Clang, however, turn import process into a minefield, where you have no idea if a "pure" operation can cause a major change the target AST. "Complete" types are really incomplete, and there is no single point at which they are all guaranteed to be finalized.

Yup, agreed. Our idea is summarized in https://discourse.llvm.org/t/rfc-lldb-more-reliable-completion-of-record-types/77442. Basically the goal is to guarantee that a call to `getDefinition`, *will* fetch the definition. This is something that Clang already does, but we just never implement (via `CompleteRedeclChain`). You're right that the "minimal import" mode was implemented for perf. reasons. Though we should probably revisit this. I think we can make LLDB use non-minimal import for record types while keeping the number of transitive imports contained. It would still require changes to the importer to be smarter about what we import, but we wouldn't have these "complete but incomplete" types floating around.

> Perhaps this approach was taken to minimize impact of external sources on the rest of Clang, to avoid introducing a concept of "not-fully-loaded-AST" everywhere. Understandable, but we have these issues now.

Possibly, I'm not entirely sure of the history here either. But afaiu external sources are a thing because of Clang modules, we re-used it for DWARF later on (or at least that's my impression). There's quite a lot of difference in the way that LLDB makes use of this API, which is where some of the tension comes from.

https://github.com/llvm/llvm-project/pull/107828