[PATCH] D50947: lambdas in modules: make PendingFakeDefinitionData a set
Steve O'Brien via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 5 09:30:41 PDT 2018
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
More information about the cfe-commits
mailing list