[Clang][NFC] Simplify initialization of OverloadCandidate objects.

[cor3ntin]

Initialize some fields of OverloadCandidate in its constructor. The goal here is try to fix read of uninitialized variable (which I was not able to reproduce)
#93430 (comment)

We should certainly try to improve the construction of OverloadCandidate further as it can be quite britle.

[llvmbot]

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

Initialize some fields of OverloadCandidate in its constructor. The goal here is try to fix read of uninitialized variable (which I was not able to reproduce)
#93430 (comment)

We should certainly try to improve the construction of OverloadCandidate further as it can be quite britle.

---

Full diff: https://github.com/llvm/llvm-project/pull/100318.diff

2 Files Affected:

• (modified) clang/include/clang/Sema/Overload.h (+2-1)
• (modified) clang/lib/Sema/SemaOverload.cpp (-19)

diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index 9d8b797af6663..2cf30d7b11709 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -998,7 +998,8 @@ class Sema;
   private:
     friend class OverloadCandidateSet;
     OverloadCandidate()
-        : IsSurrogate(false), IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
+        : IsSurrogate(false), IgnoreObjectArgument(false), TookAddressOfOverload(false),
+          IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
   };
 
   /// OverloadCandidateSet - A set of overload candidates, used in C++
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index a8d250fbabfed..7c2167c23bcd8 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -6857,10 +6857,7 @@ void Sema::AddOverloadCandidate(
   Candidate.Viable = true;
   Candidate.RewriteKind =
       CandidateSet.getRewriteInfo().getRewriteKind(Function, PO);
-  Candidate.IsSurrogate = false;
   Candidate.IsADLCandidate = IsADLCandidate;
-  Candidate.IgnoreObjectArgument = false;
-  Candidate.TookAddressOfOverload = false;
   Candidate.ExplicitCallArguments = Args.size();
 
   // Explicit functions are not actually candidates at all if we're not
@@ -7422,8 +7419,6 @@ Sema::AddMethodCandidate(CXXMethodDecl *Method, DeclAccessPair FoundDecl,
   Candidate.Function = Method;
   Candidate.RewriteKind =
       CandidateSet.getRewriteInfo().getRewriteKind(Method, PO);
-  Candidate.IsSurrogate = false;
-  Candidate.IgnoreObjectArgument = false;
   Candidate.TookAddressOfOverload =
       CandidateSet.getKind() == OverloadCandidateSet::CSK_AddressOfOverloadSet;
   Candidate.ExplicitCallArguments = Args.size();
@@ -7617,7 +7612,6 @@ void Sema::AddMethodTemplateCandidate(
     Candidate.IgnoreObjectArgument =
         cast<CXXMethodDecl>(Candidate.Function)->isStatic() ||
         ObjectType.isNull();
-    Candidate.TookAddressOfOverload = false;
     Candidate.ExplicitCallArguments = Args.size();
     if (Result == TemplateDeductionResult::NonDependentConversionFailure)
       Candidate.FailureKind = ovl_fail_bad_conversion;
@@ -7705,7 +7699,6 @@ void Sema::AddTemplateOverloadCandidate(
     Candidate.IgnoreObjectArgument =
         isa<CXXMethodDecl>(Candidate.Function) &&
         !isa<CXXConstructorDecl>(Candidate.Function);
-    Candidate.TookAddressOfOverload = false;
     Candidate.ExplicitCallArguments = Args.size();
     if (Result == TemplateDeductionResult::NonDependentConversionFailure)
       Candidate.FailureKind = ovl_fail_bad_conversion;
@@ -7886,9 +7879,6 @@ void Sema::AddConversionCandidate(
   OverloadCandidate &Candidate = CandidateSet.addCandidate(1);
   Candidate.FoundDecl = FoundDecl;
   Candidate.Function = Conversion;
-  Candidate.IsSurrogate = false;
-  Candidate.IgnoreObjectArgument = false;
-  Candidate.TookAddressOfOverload = false;
   Candidate.FinalConversion.setAsIdentityConversion();
   Candidate.FinalConversion.setFromType(ConvType);
   Candidate.FinalConversion.setAllToTypes(ToType);
@@ -8084,9 +8074,6 @@ void Sema::AddTemplateConversionCandidate(
     Candidate.Function = FunctionTemplate->getTemplatedDecl();
     Candidate.Viable = false;
     Candidate.FailureKind = ovl_fail_bad_deduction;
-    Candidate.IsSurrogate = false;
-    Candidate.IgnoreObjectArgument = false;
-    Candidate.TookAddressOfOverload = false;
     Candidate.ExplicitCallArguments = 1;
     Candidate.DeductionFailure = MakeDeductionFailureInfo(Context, Result,
                                                           Info);
@@ -8120,9 +8107,6 @@ void Sema::AddSurrogateCandidate(CXXConversionDecl *Conversion,
   Candidate.Function = nullptr;
   Candidate.Surrogate = Conversion;
   Candidate.Viable = true;
-  Candidate.IsSurrogate = true;
-  Candidate.IgnoreObjectArgument = false;
-  Candidate.TookAddressOfOverload = false;
   Candidate.ExplicitCallArguments = Args.size();
 
   // Determine the implicit conversion sequence for the implicit
@@ -8328,9 +8312,6 @@ void Sema::AddBuiltinCandidate(QualType *ParamTys, ArrayRef<Expr *> Args,
   OverloadCandidate &Candidate = CandidateSet.addCandidate(Args.size());
   Candidate.FoundDecl = DeclAccessPair::make(nullptr, AS_none);
   Candidate.Function = nullptr;
-  Candidate.IsSurrogate = false;
-  Candidate.IgnoreObjectArgument = false;
-  Candidate.TookAddressOfOverload = false;
   std::copy(ParamTys, ParamTys + Args.size(), Candidate.BuiltinParamTypes);
 
   // Determine the implicit conversion sequences for each of the

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Sirraide]

Moving more stuff into the constructor makes sense.

I’m not too familiar w/ the code around overloading, but would it be possible to actually write a proper constructor for it at this point (which would take e.g. the Function and other things as an argument) to ensure that every field is initialised, instead of just manually initialising a lot of the fields afterwards which we seem to be doing atm? Or is there a reason why we can’t do that here?

[cor3ntin]

I’m not too familiar w/ the code around overloading, but would it be possible to actually write a proper constructor for it at this point (which would take e.g. the Function and other things as an argument) to ensure that every field is initialised, instead of just manually initialising a lot of the fields afterwards which we seem to be doing atm? Or is there a reason why we can’t do that here?

We might be able to, and frankly we should but it's a slightly bigger refactor that I don't really have time to investigate
(right now addCandidate is what constructs OverloadCandidate objects, a I'm not sure adding a bunch of parameters to that would be a good thing)

[Sirraide]

We might be able to, and frankly we should but it's a slightly bigger refactor that I don't really have time to investigate

Makes sense.

[cor3ntin]

/cherry-pick 7d787df

[llvmbot]

/pull-request #100410