[Lldb-commits] [PATCH] D68326: [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: shafik, martong.
Herald added subscribers: lldb-commits, JDevlieghere, abidh, christof, rnkovacs, aprantl.
Herald added a project: LLDB.

As we figured out in D67803 <https://reviews.llvm.org/D67803>, importing declarations from a temporary ASTContext that were originally from a persistent ASTContext
causes a bunch of duplicated declarations where we end up having declarations in the target AST that have no associated ASTImporter that
can complete them.

I haven't figured out how/if we can solve this in the current way we do things in LLDB, but in the modern-type-lookup this is solvable
as we have a saner architecture with the ExternalASTMerger. As we can (hopefully) make modern-type-lookup the default mode in the future,
I would say we try fixing this issue here. As we don't use the hack that was reinstated in D67803 <https://reviews.llvm.org/D67803> during modern-type-lookup, the test case for this
is essentially just printing any kind of container in `std::` as we would otherwise run into the issue that required a hack like D67803 <https://reviews.llvm.org/D67803>.

What this patch is doing in essence is that instead of importing a declaration from a temporary ASTContext, we instead check if the
declaration originally came from a persistent ASTContext (e.g. the debug information) and we directly import from there. The ExternalASTMerger
is already connected with ASTImporters to these different sources, so this patch is essentially just two parts:

1. Mark our temporary ASTContext/ImporterSource as temporary when we import from the expression AST.
2. If the ExternalASTMerger sees we import from the expression AST, instead of trying to import these temporary declarations, check if we

can instead import from the persistent ASTContext that is already connected. This ensures that all records from the persistent source actually
come from the persistent source and are minimally imported in a way that allows them to be completed later on in the target AST.

The next step is to run the ASTImporter for these temporary expressions with the MinimalImport mode disabled, but that's a follow up patch.

This patch fixes most test failures with modern-type-lookup enabled by default (down to 73 failing tests, which includes the 22 import-std-module tests
which need special treatment).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68326

Files:
  clang/include/clang/AST/ExternalASTMerger.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/ExternalASTMerger.cpp
  lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
  lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68326.222773.patch
Type: text/x-patch
Size: 15660 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20191002/62cb0546/attachment-0001.bin>