[clang] [clang] check deduction consistency when partial ordering function templates (PR #100692)

Matheus Izvekov via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 26 20:46:01 PDT 2024


================
@@ -5554,38 +5869,50 @@ FunctionTemplateDecl *Sema::getMoreSpecializedTemplate(
     // "that is a member function with no expicit object argument".
     // Otherwise the ordering rules for methods with expicit objet arguments
     // against anything else make no sense.
-    ShouldConvert1 = Method1 && !Method1->isExplicitObjectMemberFunction();
-    ShouldConvert2 = Method2 && !Method2->isExplicitObjectMemberFunction();
-    if (ShouldConvert1) {
-      bool IsRValRef2 =
-          ShouldConvert2
-              ? Method2->getRefQualifier() == RQ_RValue
-              : Proto2->param_type_begin()[0]->isRValueReferenceType();
-      // Compare 'this' from Method1 against first parameter from Method2.
-      Obj1Ty = GetImplicitObjectParameterType(this->Context, Method1, RawObj1Ty,
-                                              IsRValRef2);
-      Args1.push_back(Obj1Ty);
-    }
-    if (ShouldConvert2) {
-      bool IsRValRef1 =
-          ShouldConvert1
-              ? Method1->getRefQualifier() == RQ_RValue
-              : Proto1->param_type_begin()[0]->isRValueReferenceType();
-      // Compare 'this' from Method2 against first parameter from Method1.
-      Obj2Ty = GetImplicitObjectParameterType(this->Context, Method2, RawObj2Ty,
-                                              IsRValRef1);
-      Args2.push_back(Obj2Ty);
-    }
+
+    bool NonStaticMethod1 = Method1 && !Method1->isStatic(),
+         NonStaticMethod2 = Method2 && !Method2->isStatic();
+
+    auto Params1Begin = Proto1->param_type_begin(),
+         Params2Begin = Proto2->param_type_begin();
+
     size_t NumComparedArguments = NumCallArguments1;
-    // Either added an argument above or the prototype includes an explicit
-    // object argument we need to count
-    if (Method1)
-      ++NumComparedArguments;
 
-    Args1.insert(Args1.end(), Proto1->param_type_begin(),
-                 Proto1->param_type_end());
-    Args2.insert(Args2.end(), Proto2->param_type_begin(),
-                 Proto2->param_type_end());
+    if ((NonStaticMethod1 && NonStaticMethod2) || FD1->isOverloadedOperator()) {
+      ShouldConvert1 =
+          NonStaticMethod1 && !Method1->hasCXXExplicitFunctionObjectParameter();
+      ShouldConvert2 =
+          NonStaticMethod2 && !Method2->hasCXXExplicitFunctionObjectParameter();
+      NumComparedArguments += 1;
+
+      if (ShouldConvert1) {
+        bool IsRValRef2 =
+            ShouldConvert2
+                ? Method2->getRefQualifier() == RQ_RValue
+                : Proto2->param_type_begin()[0]->isRValueReferenceType();
+        // Compare 'this' from Method1 against first parameter from Method2.
+        Obj1Ty = GetImplicitObjectParameterType(this->Context, Method1,
+                                                RawObj1Ty, IsRValRef2);
+        Args1.push_back(Obj1Ty);
+      }
+      if (ShouldConvert2) {
+        bool IsRValRef1 =
+            ShouldConvert1
+                ? Method1->getRefQualifier() == RQ_RValue
+                : Proto1->param_type_begin()[0]->isRValueReferenceType();
+        // Compare 'this' from Method2 against first parameter from Method1.
+        Obj2Ty = GetImplicitObjectParameterType(this->Context, Method2,
+                                                RawObj2Ty, IsRValRef1);
+        Args2.push_back(Obj2Ty);
+      }
+    } else {
+      if (NonStaticMethod1 && Method1->hasCXXExplicitFunctionObjectParameter())
+        Params1Begin += 1;
+      if (NonStaticMethod2 && Method2->hasCXXExplicitFunctionObjectParameter())
+        Params2Begin += 1;
+    }
+    Args1.insert(Args1.end(), Params1Begin, Proto1->param_type_end());
+    Args2.insert(Args2.end(), Params2Begin, Proto2->param_type_end());
----------------
mizvekov wrote:

> @mizvekov please avoid force pushing (prefer merging from main and push new commits) - otherwise github sometimes
eat comments which makes review difficult.

@cor3ntin  The comment didn't disappear by the way, it's right here.

> I previously mentioned that your changes to getMoreSpecializedTemplate probably needed explicit object parameters tests. I then realized that this was https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2834. Can you check what we do in this case? Thanks

Yeah for that case, no changes here, we are still behaving as proposed in the core issue, which is picking #2, the same as before this patch.

https://github.com/llvm/llvm-project/pull/100692


More information about the cfe-commits mailing list