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

Compound assignment is not supported. #251

Open
holdenmai opened this issue Aug 22, 2022 · 3 comments
Open

Compound assignment is not supported. #251

holdenmai opened this issue Aug 22, 2022 · 3 comments

Comments

@holdenmai
Copy link
Contributor

Currently, the below is not supported.


			var target = new Interpreter();

			var parameters = new[]{
							new Parameter(nameof(a), typeof(int)),
							new Parameter(nameof(b), typeof(int)),
							};
			Assert.DoesNotThrow(() => target.Parse("a += b", parameters));

As per the operator overloading documentation, "When a binary operator is overloaded, the corresponding compound assignment operator, if any, is also implicitly overloaded."

This can be worked around by changing our eval text to
"a = (a + b)"

@davideicardi
Copy link
Member

Another consideration is that allowing to change parameter value can cause some problems (security?). Consider the case where where two expressions are evaluated with the same parameter and the first expression change it ...
I know that from C# perspective this is valid, but maybe considering parameters read only is better in term of design.

@holdenmai
Copy link
Contributor Author

As an example where this can already be a valid use case from one of the existing unit tests (Assignment_Operator_Equal) would be to have an additional test like (to show a lot of edge cases)

x.Property1 = 4;
x.Property2 = 2;
target.Eval("x.Property1 += x.Property2 *= 10");
Assert.AreEqual(20, x.Property2);
Assert.AreEqual(24, x.Property1);

@davideicardi
Copy link
Member

Yes, I agree with you that this can be a complex discussion. In the past I have implemented this flag to disable assignements:

new Interpreter().EnableAssignment(AssignmentOperators.None);

Eventually we can do something similar ...

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

2 participants