[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

The problem may well be that you're recursively triggering deserialization of the `std` namespace from its own deserialization.

In D58920#1500246 <https://reviews.llvm.org/D58920#1500246>, @modocache wrote:

>   @@ -3401,6 +3402,22 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) {
>              return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
>                                        TypedefNameForLinkage);
>        }
>   +    if (isa<NamespaceDecl>(D) && D->getName() == "std") {
>   +      llvm::outs() << "Found std namespace: ";
>   +      D->dump();
>   +      llvm::outs() << "Merging into Sema std namespace: ";
>   +      Sema *S = Reader.getSema();
>   +      S->getStdNamespace()->dump();
>


You shouldn't call `getStdNamespace()` here, because that will trigger deserialization. You should query the `LazyDeclPtr` directly to see if there's an already-deserialized `std` namespace.

>   +      NamedDecl *Existing =
>   +          getDeclForMerging(S->getStdNamespace(), TypedefNameForLinkage);

(No need to call this, `std` isn't an anonymous struct.)

>   +      llvm::outs() << "Found Existing: ";
>   +      Existing->dump();
>   +      if (isSameEntity(Existing, D)) {
>   +        llvm::outs() << "isSameEntity is true\n";
>   +        return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
>   +                                  TypedefNameForLinkage);
>   +      }
>   +    }
>      } else {
>        // Not in a mergeable context.
>        return FindExistingResult(Reader);

You should also change `FindExistingResult::~FindExistingResult` to update `Sema::StdNamespace` to point to your newly-deserialized namespace if you didn't find a prior declaration of it, so that `Sema::getStdNamespace()` finds the deserialized namespace.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58920