dblaikie created this revision.
dblaikie added reviewers: aprantl, probinson, JDevlieghere.
dblaikie requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There's a nuanced check about when to use suffixes on these integer
non-type-template-parameters, but when rebuilding names for
-gsimple-template-names there isn't enough data in the DWARF to
determine when to use suffixes or not. So turn on suffixes always to
make it easy to match up names in llvm-dwarfdump --verify.

I /think/ if we correctly modelled auto non-type-template parameters
maybe we could put suffixes only on those. But there's also some logic
in Clang that puts the suffixes on overloaded functions - at least
that's what the parameter says (see D77598 <https://reviews.llvm.org/D77598> 
and printTemplateArguments
"TemplOverloaded" parameter) - but I think maybe it's for anything that
/can/ be overloaded, not necessarily only the things that are overloaded
(the argument value is hardcoded at the various callsites, doesn't seem
to depend on overload resolution/searching for overloaded functions). So
maybe with "auto" modeled more accurately, and differentiating between
function templates (always using type suffixes there) and class/variable
templates (only using the suffix for "auto" types) we could correctly
use integer type suffixes only in the minimal set of cases.

But maybe that's all too much fuss and we could/should just put them
everywhere always?

(more context:

- https://reviews.llvm.org/D77598#inline-1057607
- https://groups.google.com/g/llvm-dev/c/ekLMllbLIZg/m/-dhJ0hO1AAAJ )


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111477

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CodeGenCXX/debug-info-template.cpp
  clang/test/Modules/lsv-debuginfo.cpp

Index: clang/test/Modules/lsv-debuginfo.cpp
===================================================================
--- clang/test/Modules/lsv-debuginfo.cpp
+++ clang/test/Modules/lsv-debuginfo.cpp
@@ -26,14 +26,14 @@
 // CHECK: @__clang_ast =
 
 // This type isn't anchored anywhere, expect a full definition.
-// CHECK: !DICompositeType({{.*}}, name: "AlignedCharArray<4, 16>",
+// CHECK: !DICompositeType({{.*}}, name: "AlignedCharArray<4U, 16U>",
 // CHECK-SAME:             elements:
 
 // C
 // CHECK: @__clang_ast =
 
 // Here, too.
-// CHECK: !DICompositeType({{.*}}, name: "AlignedCharArray<4, 16>",
+// CHECK: !DICompositeType({{.*}}, name: "AlignedCharArray<4U, 16U>",
 // CHECK-SAME:             elements:
 
 #include <B/B.h>
Index: clang/test/CodeGenCXX/debug-info-template.cpp
===================================================================
--- clang/test/CodeGenCXX/debug-info-template.cpp
+++ clang/test/CodeGenCXX/debug-info-template.cpp
@@ -30,7 +30,7 @@
 // CHECK: ![[TCNESTED]] ={{.*}}!DICompositeType(tag: DW_TAG_structure_type, name: "nested",
 // CHECK-SAME:             scope: ![[TC:[0-9]+]],
 
-// CHECK: ![[TC]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "TC<unsigned int, 2, &glb, &foo::e, &foo::f, &foo::g, 1, 2, 3>"
+// CHECK: ![[TC]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "TC<unsigned int, 2U, &glb, &foo::e, &foo::f, &foo::g, 1, 2, 3>"
 // CHECK-SAME:                              templateParams: [[TCARGS:![0-9]*]]
 TC
 // CHECK: [[EMPTY:![0-9]*]] = !{}
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -10920,9 +10920,9 @@
     }
 
     Out << " = ";
-    Args[I].print(
-        getPrintingPolicy(), Out,
-        TemplateParameterList::shouldIncludeTypeForArgument(Params, I));
+    Args[I].print(getPrintingPolicy(), Out,
+                  TemplateParameterList::shouldIncludeTypeForArgument(
+                      getPrintingPolicy(), Params, I));
   }
 
   Out << ']';
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -983,9 +983,9 @@
   for (auto &Arg : Args.arguments()) {
     if (!First)
       OS << ", ";
-    Arg.getArgument().print(
-        PrintingPolicy, OS,
-        TemplateParameterList::shouldIncludeTypeForArgument(Params, I));
+    Arg.getArgument().print(PrintingPolicy, OS,
+                            TemplateParameterList::shouldIncludeTypeForArgument(
+                                PrintingPolicy, Params, I));
     First = false;
     I++;
   }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -247,6 +247,7 @@
   PP.SuppressInlineNamespace = false;
   PP.PrintCanonicalTypes = true;
   PP.UsePreferredNames = false;
+  PP.UseIntegerTypeSuffixesAlways = true;
 
   // Apply -fdebug-prefix-map.
   PP.Callbacks = &PrintCB;
Index: clang/lib/AST/TypePrinter.cpp
===================================================================
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -2046,9 +2046,9 @@
       if (!FirstArg)
         OS << Comma;
       // Tries to print the argument with location info if exists.
-      printArgument(
-          Arg, Policy, ArgOS,
-          TemplateParameterList::shouldIncludeTypeForArgument(TPL, ParmIndex));
+      printArgument(Arg, Policy, ArgOS,
+                    TemplateParameterList::shouldIncludeTypeForArgument(
+                        Policy, TPL, ParmIndex));
     }
     StringRef ArgString = ArgOS.str();
 
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -763,9 +763,9 @@
         StringRef Param = Params->getParam(i)->getName();
         if (Param.empty()) continue;
         TOut << Param << " = ";
-        Args.get(i).print(
-            Policy, TOut,
-            TemplateParameterList::shouldIncludeTypeForArgument(Params, i));
+        Args.get(i).print(Policy, TOut,
+                          TemplateParameterList::shouldIncludeTypeForArgument(
+                              Policy, Params, i));
         TOut << ", ";
       }
     }
Index: clang/lib/AST/DeclTemplate.cpp
===================================================================
--- clang/lib/AST/DeclTemplate.cpp
+++ clang/lib/AST/DeclTemplate.cpp
@@ -202,8 +202,9 @@
 }
 
 bool TemplateParameterList::shouldIncludeTypeForArgument(
-    const TemplateParameterList *TPL, unsigned Idx) {
-  if (!TPL || Idx >= TPL->size())
+    const PrintingPolicy &Policy, const TemplateParameterList *TPL,
+    unsigned Idx) {
+  if (!TPL || Idx >= TPL->size() || Policy.UseIntegerTypeSuffixesAlways)
     return true;
   const NamedDecl *TemplParam = TPL->getParam(Idx);
   if (const auto *ParamValueDecl =
Index: clang/lib/AST/DeclPrinter.cpp
===================================================================
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -1102,9 +1102,9 @@
     if (TemplOverloaded || !Params)
       Args[I].print(Policy, Out, /*IncludeType*/ true);
     else
-      Args[I].print(
-          Policy, Out,
-          TemplateParameterList::shouldIncludeTypeForArgument(Params, I));
+      Args[I].print(Policy, Out,
+                    TemplateParameterList::shouldIncludeTypeForArgument(
+                        Policy, Params, I));
   }
   Out << ">";
 }
@@ -1121,7 +1121,8 @@
     else
       Args[I].getArgument().print(
           Policy, Out,
-          TemplateParameterList::shouldIncludeTypeForArgument(Params, I));
+          TemplateParameterList::shouldIncludeTypeForArgument(Policy, Params,
+                                                              I));
   }
   Out << ">";
 }
Index: clang/include/clang/AST/PrettyPrinter.h
===================================================================
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -75,7 +75,7 @@
         MSVCFormatting(false), ConstantsAsWritten(false),
         SuppressImplicitBase(false), FullyQualifiedName(false),
         PrintCanonicalTypes(false), PrintInjectedClassNameWithArguments(true),
-        UsePreferredNames(true) {}
+        UsePreferredNames(true), UseIntegerTypeSuffixesAlways(false) {}
 
   /// Adjust this printing policy for cases where it's known that we're
   /// printing C++ code (for instance, if AST dumping reaches a C++-only
@@ -273,8 +273,15 @@
   /// written. When a template argument is unnamed, printing it results in
   /// invalid C++ code.
   unsigned PrintInjectedClassNameWithArguments : 1;
+
+  /// Whether to use C++ template preferred_name attributes when printing
+  /// templates.
   unsigned UsePreferredNames : 1;
 
+  /// Whether to use type suffixes (eg: 1U) on integral non-type template
+  /// parameters.
+  unsigned UseIntegerTypeSuffixesAlways : 1;
+
   /// Callbacks to use to allow the behavior of printing to be customized.
   const PrintingCallbacks *Callbacks = nullptr;
 };
Index: clang/include/clang/AST/DeclTemplate.h
===================================================================
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -203,7 +203,8 @@
   void print(raw_ostream &Out, const ASTContext &Context,
              const PrintingPolicy &Policy, bool OmitTemplateKW = false) const;
 
-  static bool shouldIncludeTypeForArgument(const TemplateParameterList *TPL,
+  static bool shouldIncludeTypeForArgument(const PrintingPolicy &Policy,
+                                           const TemplateParameterList *TPL,
                                            unsigned Idx);
 };
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to