[PATCH] D50947: lambdas in modules: make PendingFakeDefinitionData a set

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

rsmith added inline comments.


================
Comment at: lib/Serialization/ASTReaderDecl.cpp:1766
+
+    // FIXME: handle serialized lambdas
     assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?");
----------------
Did you mean to add this FIXME?


================
Comment at: lib/Serialization/ASTReaderDecl.cpp:4246
           OldDD && (OldDD->Definition != RD ||
                     !Reader.PendingFakeDefinitionData.count(OldDD));
       RD->setParamDestroyedInCallee(Record.readInt());
----------------
This does not appear to be NFC: the `FakeLoaded` vs absent-from-map distinction matters here. I think the idea here is that we still need to load the lexical contents of the declaration of the class that we pick to be the definition even if we've already found and merged another definition into it.

That said, if this //is// necessary, we should have some test coverage for it, and based on this change not breaking any of our tests, we evidently don't. :( I'll try this change out against our codebase and see if I can find a testcase.


Repository:
  rC Clang

https://reviews.llvm.org/D50947