[clang][TypePrinter] Support expression template arguments when checking defaultedness

[Michael137]

Summary

This patch adds support for TemplateArguments of kind
TemplateArgument::Expression to clang::isSubstitutedDefaultArgument.
We do so by evaluating both the Pattern and Arg expression to an
APInt, if we can, and comparing the results.

This will be useful in an upcoming change where
clang::isSubstitutedDefaultArgument gets called from clang::Sema
where the TemplateArguments are instantiated as expressions (without
being evaluted to APInt beforehand).

Testing

• Added unit-tests

[erichkeane]

I think this looks correct, but 1 thing I found.

clang/lib/AST/TypePrinter.cpp

2020

this left over from debugging?

[Michael137]

clang/lib/AST/TypePrinter.cpp

2020

Yes! Good catch!

[Michael137]

• Remove dump() leftover from debugging

[aprantl]

clang/lib/AST/TypePrinter.cpp

2018

nit: . at the end.

2019

I assume you checked that this is always non-null?

2031

Just for my own education: what's an example for a value-dependent constant integer expression?

[Michael137]

clang/lib/AST/TypePrinter.cpp

2031

There is no such case, we're checking it here because it's a pre-condition for isIntegerConstantExpr. But we can have value-dependent expressions here like sizeof(T) we so can't unconditionally call isIntegerConstantExpr

[Michael137]

clang/lib/AST/TypePrinter.cpp

2019

I didn't add a null-check because no other caller around clang does it (including isSubstitutedTemplateArgument which precedes this call here). So we've already been doing this on top-of-tree for a while in this codepath. But may be worth a followup audit. Though I suspect the assumption is that we never construct TemplateArguments with nullptr Exprs. @erichkeane may know some more about this

clang/lib/AST/TypePrinter.cpp

for (unsigned I = 0, N = TemplateArgs.size(); I != N; ++I)
         return false;
     return true;
   }
 
   // FIXME: Handle more cases.
   return false;
 }
 
+/// Evaluates the expression template argument 'Pattern' and returns true
+/// if 'Arg' evaluates to the same result.
+static bool templateArgumentExpressionsEqual(ASTContext const &Ctx,
+                                             TemplateArgument const &Pattern,
+                                             TemplateArgument const &Arg) {
+  if (Pattern.getKind() != TemplateArgument::Expression)
+    return false;
+
+  // Can't evaluate value-dependent expressions so bail early
+  Expr const *pattern_expr = Pattern.getAsExpr();
+  if (pattern_expr->isValueDependent() ||
+      !pattern_expr->isIntegerConstantExpr(Ctx))
+    return false;
+
+  if (Arg.getKind() == TemplateArgument::Integral)
+    return llvm::APSInt::isSameValue(pattern_expr->EvaluateKnownConstInt(Ctx),
+                                     Arg.getAsIntegral());
+
+  if (Arg.getKind() == TemplateArgument::Expression) {
+    Expr const *args_expr = Arg.getAsExpr();
+    if (args_expr->isValueDependent() || !args_expr->isIntegerConstantExpr(Ctx))
+      return false;
+
+    return llvm::APSInt::isSameValue(args_expr->EvaluateKnownConstInt(Ctx),
+                                     pattern_expr->EvaluateKnownConstInt(Ctx));
+  }
+
+  return false;
+}
+
 static bool isSubstitutedTemplateArgument(ASTContext &Ctx, TemplateArgument Arg,
                                           TemplateArgument Pattern,
                                           ArrayRef<TemplateArgument> Args,
                                           unsigned Depth) {
   Arg = Ctx.getCanonicalTemplateArgument(Arg);
   Pattern = Ctx.getCanonicalTemplateArgument(Pattern);
   if (Arg.structurallyEquals(Pattern))
     return true;
 
   if (Pattern.getKind() == TemplateArgument::Expression) {
     if (auto *DRE =
             dyn_cast<DeclRefExpr>(Pattern.getAsExpr()->IgnoreParenImpCasts())) {
       if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(DRE->getDecl()))
         return NTTP->getDepth() == Depth && Args.size() > NTTP->getIndex() &&
                Args[NTTP->getIndex()].structurallyEquals(Arg);
     }
   }
 
-  if (Arg.getKind() == TemplateArgument::Integral &&
-      Pattern.getKind() == TemplateArgument::Expression) {
-    Expr const *expr = Pattern.getAsExpr();
-
-    if (!expr->isValueDependent() && expr->isIntegerConstantExpr(Ctx)) {
-      return llvm::APSInt::isSameValue(expr->EvaluateKnownConstInt(Ctx),
-                                       Arg.getAsIntegral());
-    }
-  }
+  if (templateArgumentExpressionsEqual(Ctx, Pattern, Arg))
+    return true;
 
   if (Arg.getKind() != Pattern.getKind())
     return false;
 
   if (Arg.getKind() == TemplateArgument::Type)
     return isSubstitutedType(Ctx, Arg.getAsType(), Pattern.getAsType(), Args,
                              Depth);
 

clang/unittests/AST/TypePrinterTest.cpp

TEST(TypePrinter, TemplateIdWithNTTP) {
 
   ASSERT_TRUE(PrintedTypeMatches(
       Code, {"-std=c++20"}, Matcher,
       R"(ASCII<Str<52>{"this nontype template argument is too long to print"}> &&)",
       [](PrintingPolicy &Policy) {
         Policy.EntireContentsOfLargeArray = true;
       }));
 }
+
+TEST(TypePrinter, TemplateArgumentsSubstitution_Expressions) {
+  /// Tests clang::isSubstitutedDefaultArgument on TemplateArguments
+  /// that are of kind TemplateArgument::Expression
+  constexpr char Code[] = R"cpp(
+    constexpr bool func() { return true; }
+
+    template <typename T1 = int,
+              int      T2 = 42,
+              T1       T3 = 43,
+              int      T4 = sizeof(T1),
+              bool     T5 = func()
+              >
+    struct Foo {
+    };
+
+    Foo<int, 40 + 2> X;
+  )cpp";
+
+  auto AST = tooling::buildASTFromCodeWithArgs(Code, /*Args=*/{"-std=c++20"});
+  ASTContext &Ctx = AST->getASTContext();
+
+  auto const *CTD = selectFirst<ClassTemplateDecl>(
+      "id", match(classTemplateDecl(hasName("Foo")).bind("id"), Ctx));
+  ASSERT_NE(CTD, nullptr);
+  auto const *CTSD = *CTD->specializations().begin();
+  ASSERT_NE(CTSD, nullptr);
+  auto const *Params = CTD->getTemplateParameters();
+  ASSERT_NE(Params, nullptr);
+  auto const &ArgList = CTSD->getTemplateArgs();
+
+  auto createBinOpExpr = [&](uint32_t LHS, uint32_t RHS,
+                             uint32_t Result) -> ConstantExpr * {
+    const int numBits = 32;
+    clang::APValue ResultVal{llvm::APSInt(llvm::APInt(numBits, Result))};
+    auto *LHSInt = IntegerLiteral::Create(Ctx, llvm::APInt(numBits, LHS),
+                                          Ctx.UnsignedIntTy, {});
+    auto *RHSInt = IntegerLiteral::Create(Ctx, llvm::APInt(numBits, RHS),
+                                          Ctx.UnsignedIntTy, {});
+    auto *BinOp = BinaryOperator::Create(
+        Ctx, LHSInt, RHSInt, BinaryOperatorKind::BO_Add, Ctx.UnsignedIntTy,
+        ExprValueKind::VK_PRValue, ExprObjectKind::OK_Ordinary, {}, {});
+    return ConstantExpr::Create(Ctx, dyn_cast<Expr>(BinOp), ResultVal);
+  };
+
+  {
+    // Arg is an integral '42'
+    auto const &Arg = ArgList.get(1);
+    ASSERT_EQ(Arg.getKind(), TemplateArgument::Integral);
+
+    // Param has default expr which evaluates to '42'
+    auto const *Param = Params->getParam(1);
+
+    EXPECT_TRUE(clang::isSubstitutedDefaultArgument(
+        Ctx, Arg, Param, ArgList.asArray(), Params->getDepth()));
+  }
+
+  {
+    // Arg is an integral '41'
+    llvm::APInt Int(32, 41);
+    TemplateArgument Arg(Ctx, llvm::APSInt(Int), Ctx.UnsignedIntTy);
+
+    // Param has default expr which evaluates to '42'
+    auto const *Param = Params->getParam(1);
+
+    EXPECT_FALSE(clang::isSubstitutedDefaultArgument(
+        Ctx, Arg, Param, ArgList.asArray(), Params->getDepth()));
+  }
+
+  {
+    // Arg is an integral '4'
+    llvm::APInt Int(32, 4);
+    TemplateArgument Arg(Ctx, llvm::APSInt(Int), Ctx.UnsignedIntTy);
+
+    // Param has is value-dependent expression (i.e., sizeof(T))
+    auto const *Param = Params->getParam(3);
+
+    EXPECT_FALSE(clang::isSubstitutedDefaultArgument(
+        Ctx, Arg, Param, ArgList.asArray(), Params->getDepth()));
+  }
+
+  {
+    const int LHS = 40;
+    const int RHS = 2;
+    const int Result = 42;
+    auto *ConstExpr = createBinOpExpr(LHS, RHS, Result);
+    // Arg is instantiated with '40 + 2'
+    TemplateArgument Arg(ConstExpr);
+
+    // Param has default expr of '42'
+    auto const *Param = Params->getParam(1);
+
+    EXPECT_TRUE(clang::isSubstitutedDefaultArgument(
+        Ctx, Arg, Param, ArgList.asArray(), Params->getDepth()));
+  }
+
+  {
+    const int LHS = 40;
+    const int RHS = 1;
+    const int Result = 41;
+    auto *ConstExpr = createBinOpExpr(LHS, RHS, Result);
+
+    // Arg is instantiated with '40 + 1'
+    TemplateArgument Arg(ConstExpr);
+
+    // Param has default expr of '42'
+    auto const *Param = Params->getParam(1);
+
+    EXPECT_FALSE(clang::isSubstitutedDefaultArgument(
+        Ctx, Arg, Param, ArgList.asArray(), Params->getDepth()));
+  }
+
+  {
+    const int LHS = 4;
+    const int RHS = 0;
+    const int Result = 4;
+    auto *ConstExpr = createBinOpExpr(LHS, RHS, Result);
+
+    // Arg is instantiated with '4 + 0'
+    TemplateArgument Arg(ConstExpr);
+
+    // Param has is value-dependent expression (i.e., sizeof(T))
+    auto const *Param = Params->getParam(3);
+
+    EXPECT_FALSE(clang::isSubstitutedDefaultArgument(
+        Ctx, Arg, Param, ArgList.asArray(), Params->getDepth()));
+  }
+}