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

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.


================
Comment at: clang/include/clang/AST/DeclCXX.h:1987
+  void setDeductionCandidateKind(DeductionCandidateKind K) {
+    FunctionDeclBits.DeductionCandidateKind = static_cast<uint64_t>(K);
   }
----------------
shafik wrote:
> aaron.ballman wrote:
> > Er, seems a bit odd to cast an 8-bit type to a 64-bit type only to shove it into a 2-bit bit-field. I think `DeductionCandidateKind` should be an enum class whose underlying type is `int` and we cast to/from `int` as needed.
> It feels a bit weird that we are taking an enum w/ an underlying type of `unsigned char` casting it to `int` and then placing it in an unsigned bit-field. I don't have a better suggestion ATM but I wish we had something better. 
I've changed it to casting to underlying type of `DeductionCandidate` so it has one less step of conversion.


================
Comment at: clang/lib/Sema/SemaInit.cpp:15
 #include "clang/AST/DeclObjC.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
----------------
shafik wrote:
> I saw your adding headers. How did you figure out which ones were missing?
Added headers provide APIs for the new code. I guess many of them are included already indirectly (SmallVector for example). But I thought the best practice is not relying on that, so I added these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837