[AST] Add OriginalDC argument to ExternalASTSource::FindExternalVisibleDeclsByName (#123152)

Part for relanding https://github.com/llvm/llvm-project/pull/122887.

I split this to test where the pe

[AST] Add OriginalDC argument to ExternalASTSource::FindExternalVisibleDeclsByName (#123152)

Part for relanding https://github.com/llvm/llvm-project/pull/122887.

I split this to test where the performance regession comes from if
modules are not used.

show more ...

Revert "[C++20] [Modules] Support module level lookup (#122887)"

This reverts commit 7201cae106260aeb3e9bbbb7d5291ff30f05076a.

[C++20] [Modules] Support module level lookup (#122887)

Close https://github.com/llvm/llvm-project/issues/90154

This patch is also an optimization to the lookup process to utilize the
informatio

[C++20] [Modules] Support module level lookup (#122887)

Close https://github.com/llvm/llvm-project/issues/90154

This patch is also an optimization to the lookup process to utilize the
information provided by `export` keyword.

Previously, in the lookup process, the `export` keyword only takes part
in the check part, it doesn't get involved in the lookup process. That
said, previously, in a name lookup for 'name', we would load all of
declarations with the name 'name' and check if these declarations are
valid or not. It works well. But it is inefficient since it may load
declarations that may not be wanted.

Note that this patch actually did a trick in the lookup process instead
of bring module information to DeclarationName or considering module
information when deciding if two declarations are the same. So it may
not be a surprise to me if there are missing cases. But it is not a
regression. It should be already the case. Issue reports are welcomed.

In this patch, I tried to split the big lookup table into a lookup table
as before and a module local lookup table, which takes a combination of
the ID of the DeclContext and hash value of the primary module name as
the key. And refactored `DeclContext::lookup()` method to take the
module information. So that a lookup in a DeclContext won't load
declarations that are local to **other** modules.

And also I think it is already beneficial to split the big lookup table
since it may reduce the conflicts during lookups in the hash table.

BTW, this patch introduced a **regression** for a reachability rule in
C++20 but it was false-negative. See
'clang/test/CXX/module/module.interface/p7.cpp' for details.

This patch is not expected to introduce any other
regressions for non-c++20-modules users since the module local lookup
table should be empty for them.

---

On the API side, this patch unfortunately add a maybe-confusing argument
`Module *NamedModule` to
`ExternalASTSource::FindExternalVisibleDeclsByName()`. People may think
we can get the information from the first argument `const DeclContext
*DC`. But sadly there are declarations (e.g., namespace) can appear in
multiple different modules as a single declaration. So we have to add
additional information to indicate this.

show more ...

[AST] Avoid repeated map lookups (NFC) (#122858)

[AST] Use DenseMapBase::lookup (NFC)

[clang] Use llvm::erase_if (NFC)

Make iteration over the DeclContext::lookup_result safe.

The idiom:
```
DeclContext::lookup_result R = DeclContext::lookup(Name);
for (auto *D : R) {...}
```

is not safe when in the loop body we tr

Make iteration over the DeclContext::lookup_result safe.

The idiom:
```
DeclContext::lookup_result R = DeclContext::lookup(Name);
for (auto *D : R) {...}
```

is not safe when in the loop body we trigger deserialization from an AST file.
The deserialization can insert new declarations in the StoredDeclsList whose
underlying type is a vector. When the vector decides to reallocate its storage
the pointer we hold becomes invalid.

This patch replaces a SmallVector with an singly-linked list. The current
approach stores a SmallVector<NamedDecl*, 4> which is around 8 pointers.
The linked list is 3, 5, or 7. We do better in terms of memory usage for small
cases (and worse in terms of locality -- the linked list entries won't be near
each other, but will be near their corresponding declarations, and we were going
to fetch those memory pages anyway). For larger cases: the vector uses a
doubling strategy for reallocation, so will generally be between half-full and
full. Let's say it's 75% full on average, so there's N * 4/3 + 4 pointers' worth
of space allocated currently and will be 2N pointers with the linked list. So we
break even when there are N=6 entries and slightly lose in terms of memory usage
after that. We suspect that's still a win on average.

Thanks to @rsmith!

Differential revision: https://reviews.llvm.org/D91524

show more ...

Modernize llvm::Error handling in ExternalASTMerger

[lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

Summary:
As we figured out in D67803, importing declarations from a temporary ASTContext that were original

[lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

Summary:
As we figured out in 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 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.

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).

Reviewers: shafik, martong

Reviewed By: martong

Subscribers: aprantl, rnkovacs, christof, abidh, JDevlieghere, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D68326

llvm-svn: 373711

show more ...

[clang][lldb][NFC] Encapsulate ExternalASTMerger::ImporterSource

NFC preparation work for upcoming ExternalASTMerger patches.

llvm-svn: 373312

[lldb][clang][modern-type-lookup] Use ASTImporterSharedState in ExternalASTMerger

Summary:
The ExternalASTMerger should use the ASTImporterSharedState. This allows it to
handle std::pair in LLDB (bu

[lldb][clang][modern-type-lookup] Use ASTImporterSharedState in ExternalASTMerger

Summary:
The ExternalASTMerger should use the ASTImporterSharedState. This allows it to
handle std::pair in LLDB (but the rest of libc++ is still work in progress).

Reviewers: martong, shafik, a.sidorin

Subscribers: rnkovacs, christof, JDevlieghere, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D68140

llvm-svn: 373193

show more ...

[Clang] Migrate llvm::make_unique to std::make_unique

Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
o

[Clang] Migrate llvm::make_unique to std::make_unique

Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances across the monorepo.

Differential revision: https://reviews.llvm.org/D66259

llvm-svn: 368942

show more ...

[ASTImporter] Use llvm::Expected and Error in the importer API

Summary:
This is the final phase of the refactoring towards using llvm::Expected
and llvm::Error in the ASTImporter API.
This involves

[ASTImporter] Use llvm::Expected and Error in the importer API

Summary:
This is the final phase of the refactoring towards using llvm::Expected
and llvm::Error in the ASTImporter API.
This involves the following:
- remove old Import functions which returned with a pointer,
- use the Import_New functions (which return with Err or Expected) everywhere
and handle their return value
- rename Import_New functions to Import
This affects both Clang and LLDB.

Reviewers: shafik, teemperor, aprantl, a_sidorin, balazske, a.sidorin

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits, lldb-commits

Tags: #clang, #lldb

Differential Revision: https://reviews.llvm.org/D61438

llvm-svn: 360760

show more ...

Changed every use of ASTImporter::Import to Import_New

Reviewers: a.sidorin, shafik, martong, a_sidorin

Reviewed By: a_sidorin

Subscribers: rnkovacs, dkrupp, martong, Szelethus, gamesh411, cfe-com

Changed every use of ASTImporter::Import to Import_New

Reviewers: a.sidorin, shafik, martong, a_sidorin

Reviewed By: a_sidorin

Subscribers: rnkovacs, dkrupp, martong, Szelethus, gamesh411, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D55049

llvm-svn: 357913

show more ...

Remove the unused return value in ASTImporter::Imported [NFC]

Summary:
`ASTImporter::Imported` currently returns a Decl, but that return value is not used by the ASTImporter (or anywhere else)
nor i

Remove the unused return value in ASTImporter::Imported [NFC]

Summary:
`ASTImporter::Imported` currently returns a Decl, but that return value is not used by the ASTImporter (or anywhere else)
nor is it documented.

Reviewers: balazske, martong, a.sidorin, shafik

Reviewed By: balazske, martong

Subscribers: rnkovacs, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D59595

llvm-svn: 356592

show more ...

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the ne

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.

Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.

llvm-svn: 351636

show more ...

Set MustBuildLookupTable on PrimaryContext in ExternalASTMerger

Summary:
`MustBuildLookupTable` must always be called on a primary context as we otherwise
trigger an assert, but we don't ensure that

Set MustBuildLookupTable on PrimaryContext in ExternalASTMerger

Summary:
`MustBuildLookupTable` must always be called on a primary context as we otherwise
trigger an assert, but we don't ensure that this will always happen in our code right now.

This patch explicitly requests the primary context when doing this call as this shouldn't break
anything (as calling `getPrimaryContext` on a context which is its own primary context is a no-op)
but will catch these rare cases where we somehow operate on a declaration context that is
not its own primary context.

See also D54863.

Reviewers: martong, a.sidorin, shafik

Reviewed By: martong

Subscribers: davide, rnkovacs, cfe-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D54898

llvm-svn: 347863

show more ...

[ASTImporter] Added error handling for AST import.

Summary:
The goal of this change is to make the ASTImporter::Import functions return
llvm::Expected instead of the imported type.
As first part the

[ASTImporter] Added error handling for AST import.

Summary:
The goal of this change is to make the ASTImporter::Import functions return
llvm::Expected instead of the imported type.
As first part the ASTNodeImporter visit functions are updated to return with
llvm::Expected. Various `import` functions are added to ASTNodeImporter to
simplify the code and have a common place for interface towards ASTImporter
(from ASTNodeImporter). There is some temporary code that is needed before
ASTImporter is updated.

Reviewers: a.sidorin, a_sidorin, xazax.hun

Reviewed By: a_sidorin

Subscribers: dkrupp, Szelethus, rnkovacs, martong, jfb, cfe-commits

Differential Revision: https://reviews.llvm.org/D51633

llvm-svn: 344783

show more ...

[ASTImporter] Refactor Decl creation

Summary:
Generalize the creation of Decl nodes during Import. With this patch we do the
same things after and before a new AST node is created (::Create) The im

[ASTImporter] Refactor Decl creation

Summary:
Generalize the creation of Decl nodes during Import. With this patch we do the
same things after and before a new AST node is created (::Create) The import
logic should be really simple, we create the node, then we mark that as
imported, then we recursively import the parts for that node and then set them
on that node. However, the AST is actually a graph, so we have to handle
circles. If we mark something as imported (`MapImported()`) then we return with
the corresponding `To` decl whenever we want to import that node again, this way
circles are handled. In order to make this algorithm work we must ensure
things, which are handled in the generic CreateDecl<> template:
* There are no `Import()` calls in between any node creation (::Create)
and the `MapImported()` call.
* Before actually creating an AST node (::Create), we must check if
the Node had been imported already, if yes then return with that one.
One very important case for this is connected to templates: we may
start an import both from the templated decl of a template and from
the template itself.

Now, the virtual `Imported` function is called in `ASTImporter::Impor(Decl *)`,
but only once, when the `Decl` is imported. One point of this refactor is to
separate responsibilities. The original `Imported()` had 3 responsibilities:
- notify subclasses when an import happened
- register the decl into `ImportedDecls`
- initialise the Decl (set attributes, etc)
Now all of these are in separate functions:
- `Imported`
- `MapImported`
- `InitializeImportedDecl`
I tried to check all the clients, I executed tests for `ExternalASTMerger.cpp`
and some unittests for lldb.

Reviewers: a.sidorin, balazske, xazax.hun, r.stahl

Subscribers: rnkovacs, dkrupp, cfe-commits

Differential Revision: https://reviews.llvm.org/D47632

llvm-svn: 336896

show more ...

[ASTImporter] avoid warnings: unused var, switch covered

llvm-svn: 323524

[ASTImporter] Support LambdaExprs and improve template support

Also, a number of style and bug fixes was done:

* ASTImporterTest: added sanity check for source node
* ExternalASTMerger: better

[ASTImporter] Support LambdaExprs and improve template support

Also, a number of style and bug fixes was done:

* ASTImporterTest: added sanity check for source node
* ExternalASTMerger: better lookup for template specializations
* ASTImporter: don't add templated declarations into DeclContext
* ASTImporter: introduce a helper, ImportTemplateArgumentListInfo getting SourceLocations
* ASTImporter: proper set ParmVarDecls for imported FunctionProtoTypeLoc

Differential Revision: https://reviews.llvm.org/D42301

llvm-svn: 323519

show more ...

Fix -Wcast-qual warning after r314336.

llvm-svn: 314424

Add support for remembering origins to ExternalASTMerger

ExternalASTMerger has hitherto relied on being able to look up
any Decl through its named DeclContext chain. This works for
many cases, but

Add support for remembering origins to ExternalASTMerger

ExternalASTMerger has hitherto relied on being able to look up
any Decl through its named DeclContext chain. This works for
many cases, but causes problems for function-local structs,
which cannot be looked up in their containing FunctionDecl. An
example case is

void f() {
{ struct S { int a; }; }
{ struct S { bool b; }; }
}

It is not possible to lookup either of the two Ses individually
(or even to provide enough information to disambiguate) after
parsing is over; and there is typically no need to, since they
are invisible to the outside world.

However, ExternalASTMerger needs to be able to complete either
S on demand. This led to an XFAIL on test/Import/local-struct,
which this patch removes. The way the patch works is:

It defines a new data structure, ExternalASTMerger::OriginMap,
which clients are expected to maintain (default-constructing
if the origin does not have an ExternalASTMerger servicing it)
As DeclContexts are imported, if they cannot be looked up by
name they are placed in the OriginMap. This allows
ExternalASTMerger to complete them later if necessary.
As DeclContexts are imported from an origin that already has
its own OriginMap, the origins are forwarded – but only for
those DeclContexts that are actually used. This keeps the
amount of stored data minimal.

The patch also applies several improvements from review:

- Thoroughly documents the interface to ExternalASTMerger;
- Adds optional logging to help track what's going on; and
- Cleans up a bunch of braces and dangling elses.

Differential Revision: https://reviews.llvm.org/D38208

llvm-svn: 314336

show more ...

[ExternalASTMerger] Import Objective-C classes

This patch adds functionality and a test for importing Objective-C classes
and their methods.

It also adds a flag to clang-import-test to set the lang

[ExternalASTMerger] Import Objective-C classes

This patch adds functionality and a test for importing Objective-C classes
and their methods.

It also adds a flag to clang-import-test to set the language used for
parsing. This takes the same argument format as the -x option to the
driver.

Differential Revision: https://reviews.llvm.org/D35274

llvm-svn: 309014

show more ...

clang-import-test had some dead code. I did the following to eliminate it:

- eliminated error handling for the indirect CompilerInstance, which should
never generate an error as it is created;
-

clang-import-test had some dead code. I did the following to eliminate it:

- eliminated error handling for the indirect CompilerInstance, which should
never generate an error as it is created;
- added a new test for direct importation; and
- removed an unused implementation of the CompleteType() API.

This brings clang-import-test.cpp and ExternalASTMerge.cpp back to 100%
coverage on all metrics measured by DLLVM_BUILD_INSTRUMENTED_COVERAGE.

Differential Revision: https://reviews.llvm.org/D35220

llvm-svn: 307600

show more ...