shafik added inline comments.
================ Comment at: clang/include/clang/AST/DeclBase.h:1686 - /// [C++17] Only used by CXXDeductionGuideDecl. Indicates that - /// the Deduction Guide is the implicitly generated 'copy - /// deduction candidate' (is used during overload resolution). - uint64_t IsCopyDeductionCandidate : 1; + /// Only used by CXXDeductionGuideDecl. Indicates the kind + /// of the Deduction Guide that is the implicitly generated ---------------- Why remove `[C++17]` ? ================ Comment at: clang/include/clang/AST/DeclCXX.h:1987 + void setDeductionCandidateKind(DeductionCandidateKind K) { + FunctionDeclBits.DeductionCandidateKind = static_cast<uint64_t>(K); } ---------------- 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. ================ Comment at: clang/lib/Sema/SemaDecl.cpp:12647 // FIXME: Initialization should not be taking a mutable list of inits. - SmallVector<Expr*, 8> InitsCopy(DeduceInits.begin(), DeduceInits.end()); + SmallVector<Expr *, 8> InitsCopy(DeduceInits.begin(), DeduceInits.end()); return DeduceTemplateSpecializationFromInitializer(TSI, Entity, Kind, ---------------- nitpick but we seem to use `Expr*` everywhere else. ================ Comment at: clang/lib/Sema/SemaInit.cpp:15 #include "clang/AST/DeclObjC.h" +#include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" ---------------- I saw your adding headers. How did you figure out which ones were missing? ================ Comment at: clang/lib/Sema/SemaInit.cpp:513 + SmallVectorImpl<QualType> &AggrDeductionCandidateParamTypes) + : InitListChecker(S, Entity, IL, T, true, false, false, + &AggrDeductionCandidateParamTypes){}; ---------------- nit ================ Comment at: clang/lib/Sema/SemaInit.cpp:10680 + /*PartialOverloading=*/false, AllowExplicit, + ADLCallKind::NotADL, {}, true); + } else { ---------------- nit, note: PO` is not a very descriptive name. ================ Comment at: clang/lib/Sema/SemaInit.cpp:10736 + addDeductionCandidate(TD, GD, DeclAccessPair::make(TD, AS_public), + OnlyListConstructors, true); + } ---------------- ================ Comment at: clang/lib/Sema/SemaInit.cpp:10743 + addDeductionCandidate(TD, GD, DeclAccessPair::make(TD, AS_public), + OnlyListConstructors, true); + } ---------------- ================ Comment at: clang/lib/Sema/SemaInit.cpp:10763 - // C++ [over.match.ctor]p1: (non-list copy-initialization from non-class) - // For copy-initialization, the candidate functions are all the - // converting constructors (12.3.1) of that class. - // C++ [over.match.copy]p1: (non-list copy-initialization from class) - // The converting constructors of T are candidate functions. - if (!AllowExplicit) { - // Overload resolution checks whether the deduction guide is declared - // explicit for us. - - // When looking for a converting constructor, deduction guides that - // could never be called with one argument are not interesting to - // check or note. - if (GD->getMinRequiredArguments() > 1 || - (GD->getNumParams() == 0 && !GD->isVariadic())) - continue; + addDeductionCandidate(TD, GD, I.getPair(), OnlyListConstructors, false); + } ---------------- > Quoted Text Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139837/new/ https://reviews.llvm.org/D139837 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits