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

Support for standard type union discriminant with constant value #44

Merged
merged 2 commits into from
Jun 5, 2019

Conversation

bartekn
Copy link
Contributor

@bartekn bartekn commented Apr 23, 2019

Union.resolved_case doesn't support unions with standard type discriminant and constant (const) values, ex:

enum StellarValueType
{
    STELLAR_VALUE_BASIC = 0,
    STELLAR_VALUE_SIGNED = 1
};

union switch (int v)
{
case STELLAR_VALUE_BASIC:
    void;
    ...

To fix this, we are searching for identifier in namespace's enum constants if discriminant type has not been found.

EDIT, realized that it probably doesn't work for JS and Java too:

  • Update JS
  • Update Java

@bartekn bartekn requested a review from nullstyle April 23, 2019 17:49
@nullstyle
Copy link
Contributor

I'm cool with this generalized approach to solving the problem, but we should make sure to update all of the language generators and confirm they work before merging.

@bartekn
Copy link
Contributor Author

bartekn commented Apr 29, 2019

@nullstyle done. Unfortunately Java and JS solution is tricky.

In Java, the type of the value we use switch on is Integer and the types must match in cases. However, StellarValueType.STELLAR_VALUE_BASIC is not Integer and StellarValueType.STELLAR_VALUE_BASIC.getValue() is not constant (getting "Constant expression required" error).

In JS, I was getting "ChildUnion._switchOn.fromName is not a function" error because _switchOn on the union is equal xdr.int().

In both cases I ended up with rendering actual numeric values of constants (0 for STELLAR_VALUE_BASIC and 1 for STELLAR_VALUE_SIGNED). This is not ideal but is a working solution.

If you have a better idea how to solve this don't hesitate and change my code.

@bartekn bartekn merged commit 2717d40 into master Jun 5, 2019
@bartekn bartekn deleted the enum-standard-type-discriminant branch June 5, 2019 12:26
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