[PATCH] D85144: [clang] Improve Dumping of APValues

2020-10-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Ideally we should be tree-dumping the `LValue` representation rather than pretty-printing it (and similarly for member pointers and label address differences). The problem with doing so is that we can't interpret the `LValuePathEntry`s without knowing the type of the

[PATCH] D85144: [clang] Improve Dumping of APValues

2020-08-14 Thread Tyker via Phabricator via cfe-commits
Tyker updated this revision to Diff 285747. Tyker added a comment. In D85144#2205461 , @riccibruno wrote: > I agree with you that it's fine to use `printPretty` for leaves (and > additionally it would be annoying to duplicate the `LValue` case); that's

[PATCH] D85144: [clang] Improve Dumping of APValues

2020-08-09 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. I agree with you that it's fine to use `printPretty` for leaves (and additionally it would be annoying to duplicate the `LValue` case); that's what I was planning to do anyway. What I am not sure I agree with is the additional complexity to handle the

[PATCH] D85144: [clang] Improve Dumping of APValues

2020-08-07 Thread Tyker via Phabricator via cfe-commits
Tyker updated this revision to Diff 283984. Tyker marked an inline comment as done. Tyker added a comment. this a patch i had to improve dumping before your improvement to dumping. I think there is benefits to being able to dump APValues like LValues without an ASTContext even if the output

[PATCH] D85144: [clang] Improve Dumping of APValues

2020-08-07 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments. Comment at: clang/include/clang/AST/PrettyPrinter.h:222 + /// Whether null pointers should be printed as nullptr or as NULL. + unsigned UseNullptr : 1; + riccibruno wrote: > This seems to be unrelated. And anyway shouldn't

[PATCH] D85144: [clang] Improve Dumping of APValues

2020-08-03 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments. Comment at: clang/lib/AST/APValue.cpp:539 Out << '[' << Path[I].getAsArrayIndex() << ']'; -ElemTy = Ctx.getAsArrayType(ElemTy)->getElementType(); +ElemTy = cast(ElemTy.getCanonicalType())->getElementType(); }

[PATCH] D85144: [clang] Improve Dumping of APValues

2020-08-03 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. Thanks for finishing this. I don't see why you are adding `dumpPretty`; the point of the `dump` function I added is to dump the `APValue` as the tree-like structure it is. But your `dumpPretty` doesn't do that at all. Instead it is just an alias for `printPretty`.

[PATCH] D85144: [clang] Improve Dumping of APValues

2020-08-03 Thread Tyker via Phabricator via cfe-commits
Tyker updated this revision to Diff 282687. Tyker added a comment. remove unintended code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85144/new/ https://reviews.llvm.org/D85144 Files: clang/include/clang/AST/APValue.h

[PATCH] D85144: [clang] Improve Dumping of APValues

2020-08-03 Thread Tyker via Phabricator via cfe-commits
Tyker created this revision. Tyker added reviewers: rsmith, riccibruno. Herald added a project: clang. Herald added a subscriber: cfe-commits. Tyker requested review of this revision. this is required for proper testing of D63640 Repository: rG LLVM Github