[PATCH] D43494: [Modules] Fix creating fake definition data for lambdas.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.


================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2969
 
+    if (!DD && RD->isBeingDefined())
+      return nullptr;
----------------
vsapsai wrote:
> aprantl wrote:
> > Perhaps add a comment explaining what's going on in this early exit?
> While I was trying to explain this early exit, I realized one of my assumptions was incorrect. Now I'm not sure returning nullptr is the right fix, maybe assigning incomplete definition data like
> 
> ```lang=c++
> D->DefinitionData = DD;
> ReadCXXDefinitionData(*DD, D);
> ```
> is better.
> 
> I am trying to come up with a test case showing different behaviour for these 2 different fixes. But so far nothing yet. If anybody can tell which one is correct: null context for merging or not-yet-populated definition data, that would be helpful.
Looks like both fix approaches should work as in `ASTDeclReader::ReadCXXDefinitionData` we read decl only for lambda. If I'm not mistaken, for lambdas merging doesn't really make sense, so no context for merging doesn't have repercussions.


https://reviews.llvm.org/D43494