[AST] Fix printing of large 64-bit unsigned template arguments

[devajithvs]

Clang omits the ULL suffix when printing large unsigned 64-bit template arguments. This patch ensures that values with the high bit set are explicitly printed with ULL.

Based on the original patch by Axel Naumann

[llvmbot]

@llvm/pr-subscribers-clang

Author: Devajith (devajithvs)

Changes

Clang omits the ULL suffix when printing large unsigned 64-bit template arguments. This patch ensures that values with the high bit set are explicitly printed with ULL.`

Based on the original patch by Axel Naumann

---

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

2 Files Affected:

• (modified) clang/lib/AST/TemplateBase.cpp (+7-1)
• (added) clang/test/AST/ast-print-integral-template-args.cpp (+7)

diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp
index 0eef8f305fcb3..934e8f1f044f5 100644
--- a/clang/lib/AST/TemplateBase.cpp
+++ b/clang/lib/AST/TemplateBase.cpp
@@ -130,8 +130,14 @@ static void printIntegral(const TemplateArgument &TemplArg, raw_ostream &Out,
     } else
       Out << "(" << T->getCanonicalTypeInternal().getAsString(Policy) << ")"
           << Val;
-  } else
+  } else {
     Out << Val;
+    // Handle cases where the value is too large to fit into the underlying type
+    // i.e. where the unsignedness matters.
+    if (T->isBuiltinType())
+      if (Val.isUnsigned() && Val.getBitWidth() == 64 && Val.countLeadingOnes())
+        Out << "ULL";
+  }
 }
 
 static unsigned getArrayDepth(QualType type) {
diff --git a/clang/test/AST/ast-print-integral-template-args.cpp b/clang/test/AST/ast-print-integral-template-args.cpp
new file mode 100644
index 0000000000000..7bec0557a4304
--- /dev/null
+++ b/clang/test/AST/ast-print-integral-template-args.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -ast-print -std=c++17 %s -o - | FileCheck %s
+
+template <unsigned long long N>
+struct Foo {};
+
+Foo<9223372036854775810> x;
+// CHECK: template<> struct Foo<9223372036854775810ULL> {

[devajithvs]

This will help in removing one of our clang patches in ROOT: root-project/root@8dff24d

root-project@5ae1304

cc: @vgvassilev

[mizvekov]

What problem is this trying to fix here?

At first glance, I think it's a problem that we ast-print instantiations as explicit specializations. If we fix that, does your problem go away?

Secondly, I agree that if we would print an implicit instantiation as a specialization, then we should use canonical arguments, and these should be printed with the expected suffix, but this patch is too narrow focused on 64-bit unsigned.

I think the right approach here would be that if we have a converted template argument, then we might as well consider it to be canonical and print it always suffixed. If we didn't lose the as-writen argument, then we would have an expression and these would always print as-written.

I think the above change would be justified by design, but I am not sure that motivation aligns with ROOTs needs here.
It would make ast-print more consistent, but it shouldn't change semantics of the code if it round trips.

On a related note, if we do explicit specialization in source code, then we always print the argument suffixed, but we shouldn't in this case, and also the same incorrect thing happens for the declaration of the explicit instantiation.