[llvm-branch-commits] [clang] [clang] fix template argument conversion (PR #124386)

[via llvm-branch-commits]

================
@@ -5252,63 +5253,89 @@ bool Sema::CheckTemplateArgument(
         return true;
     }
 
-    switch (Arg.getArgument().getKind()) {
-    case TemplateArgument::Null:
-      llvm_unreachable("Should never see a NULL template argument here");
-
-    case TemplateArgument::Expression: {
-      Expr *E = Arg.getArgument().getAsExpr();
+    auto checkExpr = [&](Expr *E) -> Expr * {
       TemplateArgument SugaredResult, CanonicalResult;
       unsigned CurSFINAEErrors = NumSFINAEErrors;
       ExprResult Res =
           CheckTemplateArgument(NTTP, NTTPType, E, SugaredResult,
                                 CanonicalResult, PartialOrderingTTP, CTAK);
-      if (Res.isInvalid())
-        return true;
       // If the current template argument causes an error, give up now.
-      if (CurSFINAEErrors < NumSFINAEErrors)
-        return true;
+      if (Res.isInvalid() || CurSFINAEErrors < NumSFINAEErrors)
+        return nullptr;
+      SugaredConverted.push_back(SugaredResult);
+      CanonicalConverted.push_back(CanonicalResult);
+      return Res.get();
+    };
+
+    switch (Arg.getKind()) {
+    case TemplateArgument::Null:
+      llvm_unreachable("Should never see a NULL template argument here");
 
+    case TemplateArgument::Expression: {
+      Expr *E = Arg.getAsExpr();
+      Expr *R = checkExpr(E);
+      if (!R)
+        return true;
       // If the resulting expression is new, then use it in place of the
       // old expression in the template argument.
-      if (Res.get() != E) {
-        TemplateArgument TA(Res.get());
-        Arg = TemplateArgumentLoc(TA, Res.get());
+      if (R != E) {
+        TemplateArgument TA(R);
+        ArgLoc = TemplateArgumentLoc(TA, R);
       }
-
-      SugaredConverted.push_back(SugaredResult);
-      CanonicalConverted.push_back(CanonicalResult);
       break;
     }
 
-    case TemplateArgument::Declaration:
-    case TemplateArgument::Integral:
+    // As for the converted NTTP kinds, they still might need another
+    // conversion, as the new corresponding parameter might be different.
+    // Ideally, we would always perform substitution starting with sugared types
+    // and never need these, as we would still have expressions. Since these are
+    // needed so rarely, it's probably a better tradeoff to just convert them
+    // back to expressions.
+    case TemplateArgument::Integral: {
+      IntegerLiteral ILE(Context, Arg.getAsIntegral(), Arg.getIntegralType(),
+                         SourceLocation());
+      if (!checkExpr(&ILE))
+        return true;
+      break;
+    }
+    case TemplateArgument::Declaration: {
+      DeclRefExpr DRE(Context, Arg.getAsDecl(),
+                      /*RefersToEnclosingVariableOrCapture=*/false,
+                      Arg.getParamTypeForDecl().getNonReferenceType(),
+                      VK_LValue, SourceLocation());
+      if (!checkExpr(&DRE))
+        return true;
+      break;
+    }
+    case TemplateArgument::NullPtr: {
+      CXXNullPtrLiteralExpr NPLE(Arg.getNullPtrType(), SourceLocation());
+      if (!checkExpr(&NPLE))
+        return true;
+      break;
+    }
     case TemplateArgument::StructuralValue:
-    case TemplateArgument::NullPtr:
-      // We've already checked this template argument, so just copy
-      // it to the list of converted arguments.
-      SugaredConverted.push_back(Arg.getArgument());
-      CanonicalConverted.push_back(
-          Context.getCanonicalTemplateArgument(Arg.getArgument()));
+      // FIXME: Is it even possible to reach here?
+      // If this is actually used, this needs to convert the argument again.
----------------
cor3ntin wrote:

Did you try to put an assert and see if the tests pass?
Adding tests for floating points and enumerations types might also get us there

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