Skip to content

Conversation

SahilPatidar
Copy link
Contributor

@SahilPatidar SahilPatidar commented Sep 3, 2025

Reimplement value printing on top of ORC MemoryAccess. The previous
implementation only supported in-process evaluation; with this new
design it now works in both in-process and out-of-process modes.

The implementation introduces a new Value class design inspired by APValue
(This design also supports array element access via index) for capturing evaluated
values, a ReaderDispatcher for reading values from MemoryAccess, and a ValueToString
printer for converting buffers back to strings.

@SahilPatidar
Copy link
Contributor Author

@vgvassilev This is still a draft — some parts are not implemented yet (e.g. record and function types), but I shared it so you can see the idea and check if the approach looks feasible.

@SahilPatidar
Copy link
Contributor Author

And also I need a way to detect whether the interpreter is running in-process or out-of-process.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we have to cast the data when we know its type? If that's needed for out-of-process where the host and target architectures do not match, we should probably somehow do it in a separate facility because getX is much faster on matching architectures.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m working on a new design inspired by clang::APValue, intended to replace Value so it can support both execution environments:https://gist.github.com/SahilPatidar/9ba885fb29aebc68b6cf788b8937a204

Copy link

github-actions bot commented Sep 8, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@SahilPatidar
Copy link
Contributor Author

@vgvassilev Introduce new Value class design inspired by APValue.

This design also supports array element access via index.

Currently, only record types trigger issues during AST conversion of
prvalues. For example, cases like S{} in tests cause problems,
since such expressions are not directly addressable. We need to explore
whether these kinds of expressions can be made addressable, or if they
should instead be created using a new expression node and then destroyed
via their address.

@SahilPatidar
Copy link
Contributor Author

Added ValueCleanup to manage destruction of manually allocated objects.
This struct allows attaching destructor logic to Value instances.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This header is intended to be used at Interpreter's runtime and it needs to be quick to include. Can we achieve the same goals with less tokens here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed two headers, but ExecutorAddr is still directly used by ValueCleanup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance to rely on forward declarations and move everything out of line?

@SahilPatidar SahilPatidar marked this pull request as ready for review September 16, 2025 03:36
@SahilPatidar
Copy link
Contributor Author

@vgvassilev Since I don’t have much experience with AST conversion, I’m not very confident about the AstConversion part in ExprConverter. It feels a bit unstable, and I’m not sure how well it handles all record types and member pointers. Could you take a look and provide a review of that section in InterpreterValuePrinter.cpp?

class Value;

/// \struct ValueCleanup
/// \brief Encapsulates destructor invocation for REPL values.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destructor calls are currently handled by CompileDtorCall and then called subsequently when the refcount of the Value goes to zero provided it contained a non-trivial type. Is that not sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To support this both out-of-process and in-process, we need to call the destructor using the ORC callWrapper. We can obtain the address of the destructor, but it must be invoked in a way that works consistently across different execution environments.

Currently, ValueCleanup only applies to Record-type PRValues. In the previous design, calling the destructor in-process was straightforward, but it’s unclear how that would work out-of-process. The old design allocated memory directly on the Value (controller) side, which isn’t possible in an OOP context, and the same limitation applies to arrays.

Here, ValueCleanup does two things. First, it uses a runtime call via the rundtor wrapper function, which handles destructor calls. We have the *_destroyObj call that deallocates memory in both environments. Second, it makes the individual destructor call for the object: we obtain the destructor’s address using CompileDtorCall and pass Value::getAddr (the object’s address), similar to how we previously used Value::getPtr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can move ValueResultManager elsewhere since it’s related to interpreter functionality, but we still need a similar solution to what ValueCleanup provides. ValueCleanup is essentially just a wrapper around the old approach: previously we made direct calls, but now we use orc::Wrapper calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants