Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questions about ExprKind #11

Open
oovm opened this issue Apr 23, 2022 · 3 comments
Open

Questions about ExprKind #11

oovm opened this issue Apr 23, 2022 · 3 comments
Labels
question Further information is requested

Comments

@oovm
Copy link
Contributor

oovm commented Apr 23, 2022

I don't quite understand the structure of ExprKind.

  1. why do I need ExprKind<E = Expr>, can E be any other structure?
  2. why not use wrap type Number directly.
  3. Can other fields be added? eg. NumbericArray, Association

I mean something like this:

pub enum ExprKind {
    Number(Number),
    String(String),
    Symbol(Symbol),
    Normal(Normal),
    NumbericArray(NumbericArray),
    Association(Association),
}
@oovm
Copy link
Contributor Author

oovm commented Apr 23, 2022

Some possible base types

Rust Wolfram
Number(u8) Integer / "Integer8"
Number(i8) Integer / "UnsignedInteger8"
Number(BigInt) Integer
String String
Symbol Symbol
Boolean Symbol / "True" / "False"
List(VecDeque) List
Association(IndexMap) Association
NumericArray NumericArray

@ConnorGray
Copy link
Collaborator

why do I need ExprKind<E = Expr>, can E be any other structure?

This dates back to 3fdd73d, which added an ArcExpr alternative to Expr. The problem was (and still is) that Expr uses an std::rc::Rc type for doing reference counting, but Rc's cannot be shared between threads. The ArcExpr type used the std::sync::Arc type instead and was intended to be a thread-safe alternative to Expr.

In theory this distinction is still meaningful, but I was never quite certain that the Expr vs ArcExpr dichotomy was the optimal design, so I removed it before doing the first official public release of wolfram-expr, with the hope that a better solution could be found in the future. (For example, it may be that simply making Expr use Arc instead of Rc would be a reasonable ergonomic and performance tradeoff. But I think more real-world usage experience would help make that determination.)

why not use wrap type Number directly.

  1. Number is a bit abstract for a type geared towards conversion between Rust and WL, where the concrete detail of what type of number it is would be difficult to abstract over.
  • My intuition is that the cases where the programmer would want to distinguish the Number case in-and-of-itself (and not care which type of number it is specifically) are far less common than the cases where the programmer would want to specifically check if the expression is e.g a machine integer or a machine real. There wouldn't be much ergonomic benefit to having a Number variant if the very next statement in the program would typically be another match statement over the variants of that Number.
  1. The variants of ExprKind are intended to roughly line up with the basic 'primitive' types that are distinguishable in Wolfram Language.

At a practical level, the ergonomic consideration of (1) is code like:

let x: i64.= match expr {
    ExprKind::Integer(x) => x,
    _ => todo!()
};

versus:

let x: i64 = match expr {
    ExprKind::Number(Number::Integer(x)) => x,
    _ => todo!()
};

The second case is slightly more verbose, which could add up significantly when writing larger programs that deconstruct Expr instances in many places.

Can other fields be added? eg. NumericArray, Association

I think it's plausible that additional "validated" variants could be added to ExprKind in the future, though I think that bar is relatively high, and would require more experience with a specific design of a stand-alone Association or NumericArray type instance first.

A hypothetical ExprKind::Rational(Integer, Integer) is another potential variant that seems potentially compelling.

@ConnorGray ConnorGray added the question Further information is requested label May 17, 2022
@oovm
Copy link
Contributor Author

oovm commented May 17, 2022

I think using api liketry_as_i64 may better than using match xxx, these wrapping methods can eliminate structural differences.

Nested structural easier to implement features like serde.

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

No branches or pull requests

2 participants