[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
shafik added a comment. Sounds good! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53755/new/ https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL348923: [ASTImporter] Remove import of definition from
GetAlreadyImportedOrNull (authored by martong, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D53755?vs=175639=177834#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53755/new/
https://reviews.llvm.org/D53755
Files:
cfe/trunk/include/clang/AST/ASTImporter.h
cfe/trunk/lib/AST/ASTImporter.cpp
Index: cfe/trunk/include/clang/AST/ASTImporter.h
===
--- cfe/trunk/include/clang/AST/ASTImporter.h
+++ cfe/trunk/include/clang/AST/ASTImporter.h
@@ -209,7 +209,7 @@
/// Return the copy of the given declaration in the "to" context if
/// it has already been imported from the "from" context. Otherwise return
/// NULL.
-Decl *GetAlreadyImportedOrNull(Decl *FromD);
+Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
/// Import the given declaration context from the "from"
/// AST context into the "to" AST context.
Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -1580,6 +1580,9 @@
return Err;
ToD = cast_or_null(Importer.GetAlreadyImportedOrNull(D));
+ if (ToD)
+if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(D, ToD))
+ return Err;
return Error::success();
}
@@ -7745,17 +7748,12 @@
return ToAttr;
}
-Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
- llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
- if (Pos != ImportedDecls.end()) {
-Decl *ToD = Pos->second;
-// FIXME: move this call to ImportDeclParts().
-if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(FromD,
ToD))
- llvm::consumeError(std::move(Err));
-return ToD;
- } else {
+Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
+ auto Pos = ImportedDecls.find(FromD);
+ if (Pos != ImportedDecls.end())
+return Pos->second;
+ else
return nullptr;
- }
}
Expected ASTImporter::Import_New(Decl *FromD) {
Index: cfe/trunk/include/clang/AST/ASTImporter.h
===
--- cfe/trunk/include/clang/AST/ASTImporter.h
+++ cfe/trunk/include/clang/AST/ASTImporter.h
@@ -209,7 +209,7 @@
/// Return the copy of the given declaration in the "to" context if
/// it has already been imported from the "from" context. Otherwise return
/// NULL.
-Decl *GetAlreadyImportedOrNull(Decl *FromD);
+Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
/// Import the given declaration context from the "from"
/// AST context into the "to" AST context.
Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -1580,6 +1580,9 @@
return Err;
ToD = cast_or_null(Importer.GetAlreadyImportedOrNull(D));
+ if (ToD)
+if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(D, ToD))
+ return Err;
return Error::success();
}
@@ -7745,17 +7748,12 @@
return ToAttr;
}
-Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
- llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
- if (Pos != ImportedDecls.end()) {
-Decl *ToD = Pos->second;
-// FIXME: move this call to ImportDeclParts().
-if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(FromD, ToD))
- llvm::consumeError(std::move(Err));
-return ToD;
- } else {
+Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
+ auto Pos = ImportedDecls.find(FromD);
+ if (Pos != ImportedDecls.end())
+return Pos->second;
+ else
return nullptr;
- }
}
Expected ASTImporter::Import_New(Decl *FromD) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
martong added a comment. @shafik > We need to make sure we are running the lldb test suite before committing and > watching the bots to make sure the commit does not break them. I just have tested this patch on macOS and there seems to be no regression. Linux also seems to be without any regression. Very soon I am going to commit and I am going to monitor green.lab.llvm.org/green/view/LLDB/ for sign of any failure. I will revert in case of any failure asap. Thanks for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53755/new/ https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
a_sidorin accepted this revision. a_sidorin added a comment. LGTM with a nit: the call to GetAlreadyImportedOrNull is not removed but moved into ImportDeclParts. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53755/new/ https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
shafik added a comment. As pointed out in this comment in another review https://reviews.llvm.org/D44100#1311687 We need to make sure we are running the lldb test suite before committing and watching the bots to make sure the commit does not break them. The change is not purely a refactor since previously the error was consumed. Thank you Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53755/new/ https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
martong added a subscriber: teemperor. martong added a comment. Hi @shafik , Thank you for your review. I have removed the call of `getPrimaryContext`. Fortunately, we already have one patch for the `getPrimaryContext` related changes, made by @teemperor : https://reviews.llvm.org/D54898 . I have set that patch as the parent of this one (in the stack). Could you please take a look on that patch too? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53755/new/ https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
martong updated this revision to Diff 175639.
martong marked an inline comment as done.
martong added a comment.
- Remove call of 'getPrimaryContext'
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53755/new/
https://reviews.llvm.org/D53755
Files:
include/clang/AST/ASTImporter.h
lib/AST/ASTImporter.cpp
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1580,6 +1580,9 @@
return Err;
ToD = cast_or_null(Importer.GetAlreadyImportedOrNull(D));
+ if (ToD)
+if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(D, ToD))
+ return Err;
return Error::success();
}
@@ -7729,17 +7732,12 @@
return ToAttr;
}
-Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
- llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
- if (Pos != ImportedDecls.end()) {
-Decl *ToD = Pos->second;
-// FIXME: move this call to ImportDeclParts().
-if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(FromD,
ToD))
- llvm::consumeError(std::move(Err));
-return ToD;
- } else {
+Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
+ auto Pos = ImportedDecls.find(FromD);
+ if (Pos != ImportedDecls.end())
+return Pos->second;
+ else
return nullptr;
- }
}
Decl *ASTImporter::Import(Decl *FromD) {
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -198,7 +198,7 @@
/// Return the copy of the given declaration in the "to" context if
/// it has already been imported from the "from" context. Otherwise return
/// NULL.
-Decl *GetAlreadyImportedOrNull(Decl *FromD);
+Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
/// Import the given declaration context from the "from"
/// AST context into the "to" AST context.
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1580,6 +1580,9 @@
return Err;
ToD = cast_or_null(Importer.GetAlreadyImportedOrNull(D));
+ if (ToD)
+if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(D, ToD))
+ return Err;
return Error::success();
}
@@ -7729,17 +7732,12 @@
return ToAttr;
}
-Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
- llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
- if (Pos != ImportedDecls.end()) {
-Decl *ToD = Pos->second;
-// FIXME: move this call to ImportDeclParts().
-if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(FromD, ToD))
- llvm::consumeError(std::move(Err));
-return ToD;
- } else {
+Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
+ auto Pos = ImportedDecls.find(FromD);
+ if (Pos != ImportedDecls.end())
+return Pos->second;
+ else
return nullptr;
- }
}
Decl *ASTImporter::Import(Decl *FromD) {
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -198,7 +198,7 @@
/// Return the copy of the given declaration in the "to" context if
/// it has already been imported from the "from" context. Otherwise return
/// NULL.
-Decl *GetAlreadyImportedOrNull(Decl *FromD);
+Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
/// Import the given declaration context from the "from"
/// AST context into the "to" AST context.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
shafik requested changes to this revision.
shafik added a comment.
This revision now requires changes to proceed.
The rest of the changes look good.
Comment at: lib/AST/ExternalASTMerger.cpp:147
ToTag->setHasExternalLexicalStorage();
- ToTag->setMustBuildLookupTable();
+ ToTag->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToTag));
This change looks unrelated to the refactoring of `GetAlreadyImportedOrNull()`
so should probably be in another PR
Comment at: lib/AST/ExternalASTMerger.cpp:154
ToContainer->setHasExternalLexicalStorage();
- ToContainer->setMustBuildLookupTable();
+ ToContainer->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToContainer));
martong wrote:
> a_sidorin wrote:
> > Do we have to do the same for NamespaceDecls?
> Yes, I think so.
> The primary context of a `NamespaceDecl` can be other than itself:
> ```
> DeclContext *DeclContext::getPrimaryContext() {
> switch (DeclKind) {
> case Decl::TranslationUnit:
> case Decl::ExternCContext:
> case Decl::LinkageSpec:
> case Decl::Export:
> case Decl::Block:
> case Decl::Captured:
> case Decl::OMPDeclareReduction:
> // There is only one DeclContext for these entities.
> return this;
>
> case Decl::Namespace:
> // The original namespace is our primary context.
> return static_cast(this)->getOriginalNamespace();
> ```
> Consequently, we should call `setMustBuildLookupTable` only on a
> `NamespaceDecl` if we know for sure that is a primary context.
This change looks unrelated to the refactoring of `GetAlreadyImportedOrNull()`
so should probably be in another PR
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53755/new/
https://reviews.llvm.org/D53755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
martong marked 2 inline comments as done.
martong added inline comments.
Comment at: lib/AST/ExternalASTMerger.cpp:154
ToContainer->setHasExternalLexicalStorage();
- ToContainer->setMustBuildLookupTable();
+ ToContainer->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToContainer));
a_sidorin wrote:
> Do we have to do the same for NamespaceDecls?
Yes, I think so.
The primary context of a `NamespaceDecl` can be other than itself:
```
DeclContext *DeclContext::getPrimaryContext() {
switch (DeclKind) {
case Decl::TranslationUnit:
case Decl::ExternCContext:
case Decl::LinkageSpec:
case Decl::Export:
case Decl::Block:
case Decl::Captured:
case Decl::OMPDeclareReduction:
// There is only one DeclContext for these entities.
return this;
case Decl::Namespace:
// The original namespace is our primary context.
return static_cast(this)->getOriginalNamespace();
```
Consequently, we should call `setMustBuildLookupTable` only on a
`NamespaceDecl` if we know for sure that is a primary context.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53755/new/
https://reviews.llvm.org/D53755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
a_sidorin added a comment. More constants are always welcome. Comment at: lib/AST/ExternalASTMerger.cpp:154 ToContainer->setHasExternalLexicalStorage(); - ToContainer->setMustBuildLookupTable(); + ToContainer->getPrimaryContext()->setMustBuildLookupTable(); assert(Parent.CanComplete(ToContainer)); Do we have to do the same for NamespaceDecls? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53755/new/ https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
martong added a subscriber: gamesh411. martong added a comment. @shafik , @gamesh411 I think when I used arcanist to update the patch it removed you as a reviewer and a subscriber. I am really sorry about this. Fortunately the Herald script added you guys back. And once you are here, this is a gentle ping. :) Repository: rC Clang https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
martong marked an inline comment as done. martong added inline comments. Herald added a reviewer: shafik. Herald added a subscriber: gamesh411. Comment at: lib/AST/ASTImporter.cpp:7716 } } balazske wrote: > This can be simplified by removing brace characters and removing `ToD`. Good catch, changed it. Repository: rC Clang https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
martong updated this revision to Diff 174897.
martong marked an inline comment as done.
martong removed a reviewer: shafik.
martong removed a subscriber: gamesh411.
martong added a comment.
Herald added a reviewer: shafik.
- Better style without braces
Repository:
rC Clang
https://reviews.llvm.org/D53755
Files:
include/clang/AST/ASTImporter.h
lib/AST/ASTImporter.cpp
lib/AST/ExternalASTMerger.cpp
Index: lib/AST/ExternalASTMerger.cpp
===
--- lib/AST/ExternalASTMerger.cpp
+++ lib/AST/ExternalASTMerger.cpp
@@ -144,14 +144,14 @@
}
if (auto *ToTag = dyn_cast(To)) {
ToTag->setHasExternalLexicalStorage();
- ToTag->setMustBuildLookupTable();
+ ToTag->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToTag));
} else if (auto *ToNamespace = dyn_cast(To)) {
ToNamespace->setHasExternalVisibleStorage();
assert(Parent.CanComplete(ToNamespace));
} else if (auto *ToContainer = dyn_cast(To)) {
ToContainer->setHasExternalLexicalStorage();
- ToContainer->setMustBuildLookupTable();
+ ToContainer->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToContainer));
}
return To;
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1580,6 +1580,9 @@
return Err;
ToD = cast_or_null(Importer.GetAlreadyImportedOrNull(D));
+ if (ToD)
+if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(D, ToD))
+ return Err;
return Error::success();
}
@@ -7721,17 +7724,12 @@
return ToAttr;
}
-Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
- llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
- if (Pos != ImportedDecls.end()) {
-Decl *ToD = Pos->second;
-// FIXME: move this call to ImportDeclParts().
-if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(FromD,
ToD))
- llvm::consumeError(std::move(Err));
-return ToD;
- } else {
+Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
+ auto Pos = ImportedDecls.find(FromD);
+ if (Pos != ImportedDecls.end())
+return Pos->second;
+ else
return nullptr;
- }
}
Decl *ASTImporter::Import(Decl *FromD) {
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -198,7 +198,7 @@
/// Return the copy of the given declaration in the "to" context if
/// it has already been imported from the "from" context. Otherwise return
/// NULL.
-Decl *GetAlreadyImportedOrNull(Decl *FromD);
+Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
/// Import the given declaration context from the "from"
/// AST context into the "to" AST context.
Index: lib/AST/ExternalASTMerger.cpp
===
--- lib/AST/ExternalASTMerger.cpp
+++ lib/AST/ExternalASTMerger.cpp
@@ -144,14 +144,14 @@
}
if (auto *ToTag = dyn_cast(To)) {
ToTag->setHasExternalLexicalStorage();
- ToTag->setMustBuildLookupTable();
+ ToTag->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToTag));
} else if (auto *ToNamespace = dyn_cast(To)) {
ToNamespace->setHasExternalVisibleStorage();
assert(Parent.CanComplete(ToNamespace));
} else if (auto *ToContainer = dyn_cast(To)) {
ToContainer->setHasExternalLexicalStorage();
- ToContainer->setMustBuildLookupTable();
+ ToContainer->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToContainer));
}
return To;
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1580,6 +1580,9 @@
return Err;
ToD = cast_or_null(Importer.GetAlreadyImportedOrNull(D));
+ if (ToD)
+if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(D, ToD))
+ return Err;
return Error::success();
}
@@ -7721,17 +7724,12 @@
return ToAttr;
}
-Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
- llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
- if (Pos != ImportedDecls.end()) {
-Decl *ToD = Pos->second;
-// FIXME: move this call to ImportDeclParts().
-if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(FromD, ToD))
- llvm::consumeError(std::move(Err));
-return ToD;
- } else {
+Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
+ auto Pos = ImportedDecls.find(FromD);
+ if (Pos != ImportedDecls.end())
+return Pos->second;
+ else
return nullptr;
- }
}
Decl *ASTImporter::Import(Decl *FromD) {
Index: include/clang/AST/ASTImporter.h
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
balazske added inline comments. Comment at: lib/AST/ASTImporter.cpp:7716 } } This can be simplified by removing brace characters and removing `ToD`. Repository: rC Clang https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
martong added a comment. This is about to make `GetAlreadyImportedOrNull` to be a const function, as it should be naturally. This is a query function which should not initiate other imports. Repository: rC Clang https://reviews.llvm.org/D53755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull
martong created this revision.
Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
a_sidorin
Repository:
rC Clang
https://reviews.llvm.org/D53755
Files:
include/clang/AST/ASTImporter.h
lib/AST/ASTImporter.cpp
lib/AST/ExternalASTMerger.cpp
Index: lib/AST/ExternalASTMerger.cpp
===
--- lib/AST/ExternalASTMerger.cpp
+++ lib/AST/ExternalASTMerger.cpp
@@ -144,14 +144,14 @@
}
if (auto *ToTag = dyn_cast(To)) {
ToTag->setHasExternalLexicalStorage();
- ToTag->setMustBuildLookupTable();
+ ToTag->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToTag));
} else if (auto *ToNamespace = dyn_cast(To)) {
ToNamespace->setHasExternalVisibleStorage();
assert(Parent.CanComplete(ToNamespace));
} else if (auto *ToContainer = dyn_cast(To)) {
ToContainer->setHasExternalLexicalStorage();
- ToContainer->setMustBuildLookupTable();
+ ToContainer->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToContainer));
}
return To;
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1575,6 +1575,9 @@
return Err;
ToD = cast_or_null(Importer.GetAlreadyImportedOrNull(D));
+ if (ToD)
+if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(D, ToD))
+ return Err;
return Error::success();
}
@@ -7702,13 +7705,10 @@
return ToAttr;
}
-Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
- llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
+Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
+ auto Pos = ImportedDecls.find(FromD);
if (Pos != ImportedDecls.end()) {
Decl *ToD = Pos->second;
-// FIXME: move this call to ImportDeclParts().
-if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(FromD,
ToD))
- llvm::consumeError(std::move(Err));
return ToD;
} else {
return nullptr;
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -198,7 +198,7 @@
/// Return the copy of the given declaration in the "to" context if
/// it has already been imported from the "from" context. Otherwise return
/// NULL.
-Decl *GetAlreadyImportedOrNull(Decl *FromD);
+Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
/// Import the given declaration context from the "from"
/// AST context into the "to" AST context.
Index: lib/AST/ExternalASTMerger.cpp
===
--- lib/AST/ExternalASTMerger.cpp
+++ lib/AST/ExternalASTMerger.cpp
@@ -144,14 +144,14 @@
}
if (auto *ToTag = dyn_cast(To)) {
ToTag->setHasExternalLexicalStorage();
- ToTag->setMustBuildLookupTable();
+ ToTag->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToTag));
} else if (auto *ToNamespace = dyn_cast(To)) {
ToNamespace->setHasExternalVisibleStorage();
assert(Parent.CanComplete(ToNamespace));
} else if (auto *ToContainer = dyn_cast(To)) {
ToContainer->setHasExternalLexicalStorage();
- ToContainer->setMustBuildLookupTable();
+ ToContainer->getPrimaryContext()->setMustBuildLookupTable();
assert(Parent.CanComplete(ToContainer));
}
return To;
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1575,6 +1575,9 @@
return Err;
ToD = cast_or_null(Importer.GetAlreadyImportedOrNull(D));
+ if (ToD)
+if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(D, ToD))
+ return Err;
return Error::success();
}
@@ -7702,13 +7705,10 @@
return ToAttr;
}
-Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
- llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
+Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
+ auto Pos = ImportedDecls.find(FromD);
if (Pos != ImportedDecls.end()) {
Decl *ToD = Pos->second;
-// FIXME: move this call to ImportDeclParts().
-if (Error Err = ASTNodeImporter(*this).ImportDefinitionIfNeeded(FromD, ToD))
- llvm::consumeError(std::move(Err));
return ToD;
} else {
return nullptr;
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -198,7 +198,7 @@
/// Return the copy of the given declaration in the "to" context if
/// it has already been imported from the "from" context. Otherwise return
/// NULL.
-Decl
