qir-alliance / pyqir

PyQIR is a set of APIs for generating, parsing, and evaluating Quantum Intermediate Representation (QIR).
https://qir-alliance.github.io/pyqir
MIT License
54 stars 24 forks source link

Rename `Ref` to `ResultRef` #78

Closed bamarsha closed 2 years ago

bamarsha commented 2 years ago

Following up from our design discussion of result variables, I think Ref may accidentally be a good name for this concept, if we view it as a PyQIR-specific abstraction of a reference cell, and not an LLVM pointer. (Pointer would be a separate type from Ref if we expose those.) See e.g. Rust RefCell or F# reference cells. The key being that we have enough information at compile time to statically erase them into a series of SSA variables, so they don't need to be represented in the IR at all.

This PR updates how the documentation describes Ref.

idavis commented 2 years ago

This documentation technically makes sense from a theory perspective. At the same time, it might not help clarify the purpose of this class for the intended python user. Instead, changing the name and description to be result specific would match the way it's used in the API.

/cc @samarsha @swernli

swernli commented 2 years ago

I agree with Ian here. The description of Ref is conceptually right, but in this case they are only used as results and I think it's worth naming them something specific and tying them to that usage. Further, the language concepts we are familiar with here (LLVM for the SSA side, Rust/F# for ref cell semantics) are unfamiliar to the target python audience such that the comparison is likely not that helpful to their understanding.

bamarsha commented 2 years ago

How about ResultRef?

swernli commented 2 years ago

I think ResultRef works, assuming that we don't lean as much on the reference cell comparison in the description.

bamarsha commented 2 years ago

I'm not sure what you had in mind - I still want to make clear that this is a cell or a container for a result, and not a result itself. But I simplified the description so it doesn't mention the implementation details.