[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

[Yuanfang Chen via Phabricator via cfe-commits]

ychen marked 9 inline comments as done.
ychen added inline comments.


================
Comment at: clang/lib/Sema/SemaInit.cpp:10338
+                    Context.getRValueReferenceType(ElementTypes[i]);
+              else if (isa<StringLiteral>(ListInit->getInit(i)))
+                // This deviates from the wording which is incorrect.
----------------
cor3ntin wrote:
> I think we need to support `A{("Hello")};` so we probably need to get rid of parentheses (maybe other implicit nodes?)
Agreed. Fixed.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:2590-2591
+  // constructors into deduction guides.
+  // FIXME: Add a kind for this to give more meaningful diagnostics. But can
+  // this substitution process actually fail?
+  InstantiatingTemplate BuildingDeductionGuides(*this, Loc, Template);
----------------
cor3ntin wrote:
> Maybe we should have an asserton !isInvalid then?
Looked at this a little bit more. It can fail (D46446). So I updated the comments in a few places.


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:368
       NewPack.push_back(Merged);
+        ++YA;
+      } else {
----------------
cor3ntin wrote:
> I think you can do that in the for loop as it was before
Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837