[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 3 14:23:01 PST 2021

rsmith added inline comments.

Comment at: clang/lib/AST/APValue.cpp:628-629
+static bool TryPrintAsStringLiteral(raw_ostream &Out, const ArrayType *ATy,
+                                    const APValue *Data, size_t Size) {
+  if (Size == 0)
Is there anything you can factor out of `StringLiteral::outputString` and reuse here? At least the single-character printing code seems like something we should try to not overly duplicate.

Comment at: clang/lib/AST/APValue.cpp:637-639
+  // Nothing we can do about a sequence that is not null-terminated
+  if (!Data[--Size].getInt().isZero())
+    return false;
We should drop all trailing zeroes here, because array initialization from a string literal will reconstruct them.

Comment at: clang/lib/AST/APValue.cpp:641-643
+  constexpr size_t MaxN = 36;
+  char Buf[MaxN * 2 + 3] = {'"'}; // "At most 36 escaped chars" + \0
+  auto *pBuf = Buf + 1;
Prefer `llvm::SmallString<N>` and `push_back`.

Comment at: clang/lib/AST/APValue.cpp:645
+  // Better than printing a two-digit sequence of 10 integers.
+  StringRef Ellipsis;
...except that some callers want output that uniquely identifies the template argument, and moreover some callers want output that is valid C++ code that produces the same template-id.

It'd be better to do this behind a `PrintingPolicy` flag that the diagnostics engine sets. But we'd want to turn that flag off during template type diffing.

Comment at: clang/lib/AST/APValue.cpp:902
         if (I == 10) {
           // Avoid printing out the entire contents of large arrays.
+          Out << "...}";
(The existing code is also doing elision here that will be inappropriate for some callers. If you add a `PrintingPolicy` to unambiguously print template arguments, it should disable this check too.)

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list