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

Zhihao Yuan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 3 15:32:33 PST 2021


lichray 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)
----------------
rsmith wrote:
> 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.
The goals of the two routines are different. `StringLiteral::outputString` must output something that can parse back as a string literal, even it doesn't decode UTF-8 right now; the new routine in this patch doesn't have to -- it can give up anytime it wants.

I wish `switch` statements were more composable. But if you don't mind, I can add a function in `CharInfo.h` to test and get the C-style escaped char and retire the outmost `switch` statements in those two routines.


================
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;
----------------
rsmith wrote:
> We should drop all trailing zeroes here, because array initialization from a string literal will reconstruct them.
Are you sure? `MyType<{""}>` and `MyType<{"\0\0\0"}>` are different types.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115031/new/

https://reviews.llvm.org/D115031



More information about the cfe-commits mailing list