[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

[Jun Zhang via Phabricator via cfe-commits]

junaire marked an inline comment as done.
junaire added inline comments.


================
Comment at: clang/include/clang/Interpreter/Value.h:98
+  QualType getType() const;
+  Interpreter &getInterpreter() const;
+
----------------
sgraenitz wrote:
> Can we make this private and try not to introduce more dependencies on the interpreter instance?
> 
> The inherent problem is that we will have to wire these through Orc RPC in the future once we want to support out-of-process execution.
Ughh, I don't understand can you elaborate a bit? We can't make these accessors private since they are meaningful API?  it will be used intensively in `ValuePrinter.cpp` (the second patch), and the end users may want to use them as well.


================
Comment at: clang/include/clang/Interpreter/Value.h:143
+  /// Values referencing an object are treated as pointers to the object.
+  template <typename T> T castAs() const { return CastFwd<T>::cast(*this); }
+
----------------
aaron.ballman wrote:
> This doesn't match the usual pattern used in Clang where the `get` variant returns `nullptr` and the `cast` variant asserts if the cast would be invalid. Should we go with a similar approach?
These APIs are adopted in Cling and Cling will remove the old implementation and use the upstream version of the value printing feature at some point in the future. So @v.g.vassilev hope we can keep these APIs close to Cling's variants as much as we can. I don't have a strong opinion here, what's your take here? @v.g.vassilev 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215