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.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115031/new/ https://reviews.llvm.org/D115031 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits