-
Notifications
You must be signed in to change notification settings - Fork 156
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
Give a more natural names to the Enum-related AST nodes #224
base: master
Are you sure you want to change the base?
Conversation
Not a big fan of proposed naming too. "Cast" is pretty vague (cast from what to what? to enum or from enum to something else?), "variant" is also very vague and invokes thoughts of this variant type or that variant type, neither of which seem to be closely related to getting enum object from a text label. |
No more than Compare: // C++
enum Enum { ... }
int data;
// cast from type of `data` to `Enum`
casted = static_cast<Enum>(data);
// ^^^^ ~~~~
// to from // Scala
// cast from type of `data` to `Enum`
casted = Ast.expr.EnumCast(Ast.identifier("Enum"), data)
// ^^^^ ~~~~
// to from
This name from Rust background, where elements of I don't think it's worth considering the AST node type as an operation ("getting enum object from a text label"). The arguments of |
To me, I agree with @GreyCat that The fact that Rust uses the word "variant" for enum entries is actually somewhat confusing, because as @GreyCat pointed out, https://en.wikipedia.org/wiki/Tagged_union lists it as a possible alias of the entire "tagged union" type as a whole (and for example the C++ ecosystem seems to have adopted this, seeing |
Also document that nodes and related
AbstractTranslator
methods.Renames:
Ast.expr.EnumById
Ast.expr.EnumCast
Ast.expr.EnumByLabel
Ast.expr.EnumVariant