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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D50947: m... Steve O'Brien via Phabricator via cfe-commits
    • [PATCH] D509... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D509... Steve O'Brien via Phabricator via cfe-commits
    • [PATCH] D509... Steve O'Brien via Phabricator via cfe-commits
    • [PATCH] D509... Steve O'Brien via Phabricator via cfe-commits

Reply via email to