Page MenuHomePhabricator

[clang-repl] Implement Value pretty printing
Needs ReviewPublic

Authored by junaire on Mar 24 2023, 6:35 AM.



This patch is the third part of the below RFC:

  • It implements Value::dump to enable pretty printing. For example:

clang-repl> int x = 42;
clang-repl> x
(int) 42

It also works for STL facilities:

clang-repl> #include <vector>
clang-repl> std::vector<int> v{1,2,3};
clang-repl> v
(std::vector<int> &) { 1, 2, 3 }

  • Users can also provide their own pretty printers by writing an

overload of PrintValueRuntime(T*). For example:

clang-repl> struct S{};
clang-repl> std::string PrintValueRuntime(S* s) { return "My printer!"; }
clang-repl> S{}
clang-repl> (S) My printer!

Signed-off-by: Jun Zhang <>

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
junaire updated this revision to Diff 518458.May 1 2023, 8:42 AM

Fix AutoType sugar


Looks like below diff fixes this:

diff --git a/clang/lib/Interpreter/InterpreterUtils.cpp b/clang/lib/Interpreter/InterpreterUtils.cpp
index 05e6be0e9df2..da5da55e8e95 100644
--- a/clang/lib/Interpreter/InterpreterUtils.cpp
+++ b/clang/lib/Interpreter/InterpreterUtils.cpp
@@ -425,7 +425,7 @@ QualType GetFullyQualifiedType(QualType QT, const ASTContext &Ctx) {
   // Strip deduced types.
   if (const auto *AutoTy = dyn_cast<AutoType>(QT.getTypePtr())) {
     if (!AutoTy->getDeducedType().isNull())
-      return GetFullyQualifiedType(AutoTy->getDeducedType(), Ctx);
+      return GetFullyQualifiedType(AutoTy->getDeducedType().getDesugaredType(Ctx), Ctx);
junaire updated this revision to Diff 520913.Tue, May 9, 11:07 PM

Remove unused code.

junaire updated this revision to Diff 520914.Tue, May 9, 11:10 PM

Add some comments

junaire updated this revision to Diff 520998.Wed, May 10, 8:04 AM

Export symbols in Windows.

junaire updated this revision to Diff 521961.Sat, May 13, 9:06 PM

Don't use C++17 because Clang on Windows is not default to that :(

junaire updated this revision to Diff 521962.Sat, May 13, 9:21 PM

Add macro guard.

junaire updated this revision to Diff 521981.Sun, May 14, 1:20 AM
  • include <tuple> to avoid incorrect lookup on Windows
  • Add -Xcc -fno-delayed-template-parsing to fix failure on Windows
junaire updated this revision to Diff 521985.Sun, May 14, 3:38 AM

Make and use our own std::void_t

Invite more people to the party :)

junaire updated this revision to Diff 524093.Sun, May 21, 4:22 AM

add caas namespace

I believe this is heading in a good direction. Here are some comments from a partial review. Also perhaps we should extend the patch description (future commit message) to capture the idea of the pretty-printing including the dispatch, the approach how we build the AST and how to write a custom printer.


We should add some documentation for users how to write a pretty-printer for a custom class.

We do not seem to support the multiple inheritance case where one of the base classes has a pretty-printer and the other does not. See the explanation in Cling. It is okay to not have it at the moment but we should include a FIXME here saying that we do not yet support it.


Similarly to the way we rebuild the AST during template instantiation, when we are synthesizing AST we do not need the lexical scope information generally.

Removing this need for the parser here will allow us to remove Interpreter::getParser which we should not expose as an API at that stage.


I'd prefer to inline to body of CompileDecl here instead of exposing it in the interpreter class.


We should check if there is a better way to export a subset of symbols through the llvm build system. Maybe @lhames, @sunho or @sgraenitz know how.

I didn't have the chance to complete my review (this is a pretty massive change), but here are some in-progress comments.


const mismatch here -- should be an overload set where the const member function returns a const reference and the non-const member function returns the non-const reference. (This is true in general, I see we've got other such functions that are const-qualified but returning non-const references.)

(Not critical to fix, but would be a nice follow-up NFC to correct these sorts of things; retro-fitting const correctness is hard, so we should strive for const correct by default for new code whenever possible.)


er, not certain the best way to repair this, but wrapping the comment isn't it. Maybe drop the "Incremental Compilation and Execution" bit?


You should use reserved identifiers where possible so as not to conflict with user-defined macros.


I wonder if it makes some degree of sense to use macros to declare these, so that it's easier to see there's a consistent pattern and which types are supported. e.g.,

#define __DECL_PRINT_VALUE_RUNTIME(type) __REPL_EXTERNAL_VISIBILITY std::string PrintValueRuntime(const type *__Ptr)



I also wonder: should this header have a C interface so it can be used from a C context, or is the repl context in which this is included only ever C++?


(and so forth in this header -- basically, every identifier should be in the reserved namespace unless it's the public API being exposed.)


Any way to make this take a const Decl * instead?


I'll stop commenting on this sort of thing; you should apply this sort of cleanup to the whole patch.


So the first case is for handling enumerations and otherwise we expect a structure or a union? Similar question below as well.

junaire updated this revision to Diff 525420.Wed, May 24, 8:22 PM
junaire marked 15 inline comments as done.

Address comments.

junaire added inline comments.Wed, May 24, 8:24 PM

DeclGroupRef in line 439 asks a non-const Decl*, should we use const Decl* and const cast it?


Where should I put the doc? Value.cpp is the implementation and it's unlikely for regular users to read it.

junaire marked an inline comment as done.Wed, May 24, 8:35 PM
junaire updated this revision to Diff 525425.Wed, May 24, 8:35 PM

Remove Interpereter::getParser + More clean up

v.g.vassilev added inline comments.Sun, May 28, 5:07 AM

We should move this routine to its user in maybe ValuePrinter.cpp.


I wonder if that's related to in some way? That is, does D151620 fix our exports and this section becomes redundant?

junaire updated this revision to Diff 526383.Sun, May 28, 11:00 PM

Make CreateUniqName a static helper

junaire added inline comments.Sun, May 28, 11:02 PM

I haven't tested that but I don't think so. The patch you posted is about fixing issues on particular platforms like MinGW.

junaire marked an inline comment as done.Sun, May 28, 11:02 PM
junaire edited the summary of this revision. (Show Details)Sun, May 28, 11:10 PM
v.g.vassilev added inline comments.Sun, May 28, 11:34 PM

Is that used?


Let's add a FIXME here. The problem CompileDecl and GenModule intends to solve is that when we synthesize AST we need to inform the rest of the Clang infrastructure about it and attach the produced llvm::Module to the JIT so that it can resolve symbols from it.

In cling that is solved with a RAII object which marks a scope where the synthesis happens and takes care to inform the rest of the infrastructure. We need something similar and a little more robust maybe. Something like ASTSynthesisRAII which ultimately hides this machinery especially from the public API.


All of the PrintValueRuntime seem to be duplicating code. We could use the preprocessor to expand these to but it would require some changes which I believe could be done as a separate patch:

  • Perhaps we should be compatible with cling here in terms of naming as that's a public API - there I believe we use printValue.
  • We should try to leverage TemplateBase.cpp::printIntegral for integral types.
  • We should not return std::string here but a const char* and we should provide somewhere storage for them. That would probably make porting to embedded systems easier since on many the <string> header is hard to support.

I believe it would be enough to have a fixme for now.


We could use the preprocessor the way we do in Value.cpp to expand this dispatcher.

v.g.vassilev added inline comments.Sun, May 28, 11:38 PM

Please add the llvm preamble.

junaire updated this revision to Diff 526566.Tue, May 30, 3:19 AM
junaire marked 2 inline comments as done.

Add LLVM preamble

junaire added inline comments.Tue, May 30, 3:48 AM

I can't really get the point. I agree we shouldn't expose Interpreter::GenModule as a public interface but I don't understand the solution.

  • I'd propose using PrintValueRuntime here. This is because we have used it across RFC and other documentation so I don't want to change it :(

We should not return std::string here but a const char* and we should provide somewhere storage for them. That would probably make porting to embedded systems easier since on many the <string> header is hard to support.

  • That really touches the dark corner of our design. On the one hand, we try to keep Value itself as lightweight as possible, on the other hand, we use heavy headers like <string> and <type_traits> in value pretty printing. While this is awkward, we can hardly avoid it because the implementation needs to see the definition of these types to know how to print it. So how can we do this on resources limited systems? My point is that perhaps we can keep it as is, because it's very hard to measure how expensive the feature is on the targeting platform. If the system can afford this, then we don't need to worry about anything, but if the system can't even use STL itself, presumably it shouldn't use value printing at all! Note that this doesn't conflicts with the lightness of the value runtime as it SHOULD add as little overhead as possible no matter whether the host can afford it or not.

We actually can't because it doesn't strictly match everything. For example Char_S and SChar both map to V.getSChar() IIUC?