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

generator for SizeOp assumes to be used with SimpleTypes #305

Open
fvollmer opened this issue Dec 4, 2019 · 5 comments
Open

generator for SizeOp assumes to be used with SimpleTypes #305

fvollmer opened this issue Dec 4, 2019 · 5 comments

Comments

@fvollmer
Copy link
Contributor

fvollmer commented Dec 4, 2019

SizeOp always generates code like BigInteger.valueOf(node.size()). This only works with SimpleTypes.

With custom types this may fail, since

fun myfunction(a: collection<number>, b: collection<number>) = a.size + b.size

could be generated to:

int res = BigInteger.valueOf(a.size()) + BigInteger.valueOf(b.size());
@wsafonov
Copy link
Contributor

Not quite sure what generator you are talking about.

In case you have you own custom type, you would need to implement your own generator for all operations that support your type. In case of SimpleTypes (o.i.core.expr.simpleType), numbers are actually generated to BigIntegers in java generator. And the expression you mentioned would be in turn correctly generatored to this:

Number res = AH.add(BigInteger.valueOf(a.size()), BigInteger.valueOf(b.size()));

because there is generator for PlusExpression that triggers when operands are of NumberType from simpleTypes.
So maybe you can provide some additional details regarding your case of generation.

@fvollmer
Copy link
Contributor Author

Thank you for your comments. I'm more than happy to provide some additional details.

The generator I'm talking about is http://127.0.0.1:63320/node?ref=r%3A4243557f-1c7a-4d6b-953a-807576e4bee7%28org.iets3.core.expr.genjava.base%40generator%29%2F5005695164079175011

grafik

This generator assumes, that numbers will be represented as BigInteger. I think it would be better to do something like:
grafik

Now the generator uses the Primitive Type Mapper (PTF) to generate the correct number literal.
This way the generator wouldn't depend on the SimpleType language and would be compatible with custom Type languages.

@wsafonov
Copy link
Contributor

Ok, maybe I've misinterpreted your request. So currently the java generator is not built with an available easy replacement option for java runtime types. That means for IntegerType for example that everywhere where we expect an expression of IntegerType we treat its corresponding generated java code as BigInteger. In order to support different runtime java type, you would need to replace generation rules for all expressions and operations that expect or return IntegerType. And same for RealTypes. That will be a long list to consider.

Now to your code suggestion. What you are trying to do is to generate java code that would return NumberLiteral node at runtime instead of a java representation for integer like BigInteger. So basically you have model element instead of its runtime representation in the runnable code. That's not what you want to have in the generated java code. The logical issue about that code that even if you'd try to make the generator reduce that NumberLiteral to the correct java representation, that won't be possible during the generation time, because the actual result value of the .size operation would be known only at runtime (and you cannot use it to put into the NumberLiteral during generation time). I hope you see the issue now.

We have chosen BigInteger/BidDecimal for the numeric types, because no other options in Java would give us the required ability to perform numeric computations with arbitrary precision w/o rounding issues and process integer values with unknown length. This way the generated code remains open to any possible application where these factors play an important role. So maybe you can explain or describe your context a bit more so I can understand why you want to exchange current java representation for numeric types. But in general I think, we won't consider the requirement for exchangable java runtime types for numeric types anytime soon, because it would add too much overhead to the generator implementation w/o clear need of that.

@fvollmer
Copy link
Contributor Author

You are correct, my code suggestion can't work.

Maybe it would be useful to add a new function to the Primitive Type Mapper like: PTF.createNumberObject(). For SimpleTypes this function would create BigInteger/BigDecimal and other Types could create an Integer/Double or whatever else. The overhead shouldn't be too big.

@wsafonov
Copy link
Contributor

It's not enough just to encapsulate creation of java objects behind some pluggable factory method. Think about all other code to be generated that has to deal with numbers. If on the level of generated java code you don't know what type of object you could get, you would need to provide some generic conversion mechanism or runtime handling which would add some additional runtime overhead and probably would cost you performance. Or you could handle the runtime variability during the generation time - then again you would need to consider every single case where numeric types can be involved and structure your generators accordingly so that flexible exchange of runtime types would become possible. Not saying that's it would be too hard to implement, but it will add a lot of additional effort especifally for the maintenance of generators. And because we don't have any requirement for that kind of flexibility right now, we definitely won't follow that direction at least for the next time.

Of course, if you definitely need this feature for some reason, feel free to try to extend the generation framework on your own. You can then always ask questions on the MPS Slack where also some other experienced MPS users could help on specific issues.

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

No branches or pull requests

3 participants