cor3ntin added a comment.
Hey. Thanks a lot for working on this.
I did a first pass, looking at mostly style issues, looking at conformance will
probably take me a lot more time, but i think this looks pretty good overall
================
Comment at: clang/include/clang/AST/DeclCXX.h:1911
setRangeEnd(EndLocation);
- setIsCopyDeductionCandidate(false);
+ setDeductionCandidateKind(DeductionCandidateKind::Normal);
}
----------------
I'm wondering if the constructor should take a `DeductionCandidateKind`
defaulted to normal here. All the places where it's set seem to be immediately
after construction.
================
Comment at: clang/include/clang/AST/DeclCXX.h:1953-1959
bool isCopyDeductionCandidate() const {
- return FunctionDeclBits.IsCopyDeductionCandidate;
+ return getDeductionCandidateKind() == DeductionCandidateKind::Copy;
+ }
+
+ bool isAggregateDeductionCandidate() const {
+ return getDeductionCandidateKind() == DeductionCandidateKind::Aggregate;
}
----------------
I'm not sure how useful these things are, isAggregateDeductionCandidate is only
used once
================
Comment at: clang/lib/Sema/SemaInit.cpp:39
#include "llvm/Support/raw_ostream.h"
+#include <optional>
----------------
This does not seems used
================
Comment at: clang/lib/Sema/SemaInit.cpp:312
NoInitExpr *DummyExpr = nullptr;
+ SmallVector<QualType, 8> *AggrDeductionCandidateParamTypes = nullptr;
----------------
I think using optional here would make more sense, I guess that's why you
included it.
Should it be SmallVectorImpl, such that only the caller has to bake in a size?
================
Comment at: clang/lib/Sema/SemaInit.cpp:1098-1099
maxElements = T->castAs<VectorType>()->getNumElements();
+ else if (T->isDependentType())
+ maxElements = 1;
else
----------------
Can you had a comment about that case? I'm not sure i understand what scenario
we are handling
================
Comment at: clang/lib/Sema/SemaInit.cpp:2167-2168
// If we have any base classes, they are initialized prior to the fields.
- for (auto &Base : Bases) {
+ for (auto I = Bases.begin(), E = Bases.end(); I != E; ++I) {
+ auto &Base = *I;
Expr *Init = Index < IList->getNumInits() ? IList->getInit(Index) :
nullptr;
----------------
Maybe using `enumerate` + a range for loop here would be cleaner?
================
Comment at: clang/lib/Sema/SemaOverload.cpp:7220
MethodTmpl, ExplicitTemplateArgs, Args, Specialization, Info,
- PartialOverloading, [&](ArrayRef<QualType> ParamTypes) {
+ PartialOverloading, false, [&](ArrayRef<QualType> ParamTypes) {
return CheckNonDependentConversions(
----------------
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
