[PATCH] D140002: [C++20] [Modules] Merging of lambda types in deserialization

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

This is my first and pretty shallow review pass. Need to read the standard more thoroughly to be more useful. Others are welcome to chime in (and as I'll be on vacation are encouraged to chime in).

What do you mean by

> Due to we don't care about merging unnamed entities before, now it is pretty problematic to fix it in a well formed.

In ASTReaderDecl.cpp we have a bunch of code to deal with anonymous decls, that's why I'm asking. Though I haven't tried to use any of that for your current problem.

Few stylistic nits:

- most of tests in "clang/test/Modules" start with a lowercase and use kebab-naming-style (sometimes with underscores);
- can you please use more descriptive names in tests? Currently hunting for 1-character difference isn't fun and doesn't convey the purpose and intention of the test;
- is it possible to reuse some of the testing code because trying to find differences and what they mean is time-consuming.

Looks like not all branches in the added code are covered by tests but I need more time to play with the code.



================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2222
+  if (TagDecl *Existing = findExistingLambda(D))
+    mergeRedeclarable(D, Existing, Redecl);
+}
----------------
How does this `mergeRedeclarable` work with other `mergeRedeclarable` calls in `ASTDeclReader::VisitCXXRecordDeclImpl`? Are multiple calls OK, do you expect it never to happen or something else?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140002/new/

https://reviews.llvm.org/D140002