elsteveogrande added inline comments.
================
Comment at: lib/Serialization/ASTReaderDecl.cpp:1766
+
+ // FIXME: handle serialized lambdas
assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?");
----------------
rsmith wrote:
> Did you mean to add this FIXME?
Yup: to indicate this is assert about `faked up lambda definition` will be
addressed in an upcoming diff, i.e., fixing the lambda issues :) See D50948,
D50949
================
Comment at: lib/Serialization/ASTReaderDecl.cpp:4246
OldDD && (OldDD->Definition != RD ||
!Reader.PendingFakeDefinitionData.count(OldDD));
RD->setParamDestroyedInCallee(Record.readInt());
----------------
rsmith wrote:
> 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.
Thanks for checking this! Yup I missed this; count would be different and
therefore this changes logic, which surprisingly didn't break things as far as
I could see. I'll change this back to `Fake` vs. `FakeLoaded`.
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
