Lambda Invoke does not update arguments in the parameters array

[holdenmai]

Understandably, the Lambda class is essentially a wrapper around the parsed delegate due to the way we treat the parameter creation.

However, the way we currently implement the Invoke, the following fails.

		[Test]
		public void When_lambda_is_invoked_byref_parameters_are_updated_on_invoke()
		{
			var a = 1;
			var b = 2;
			var c = 3;
			var d = 4;
			var e = 5;

			var parameters = new[]{
							new Parameter(nameof(a), typeof(int).MakeByRefType()),
							new Parameter(nameof(b), typeof(int).MakeByRefType()),
							new Parameter(nameof(c), typeof(int).MakeByRefType()),
							new Parameter(nameof(d), typeof(int).MakeByRefType()),
							new Parameter(nameof(e), typeof(int).MakeByRefType()),
							};

			var args = new object[]
			{
				a,
				b,
				c,
				d,
				e
			};

			a += b *= c -= d /= e %= 3;

			var target = new Interpreter();

			var lambda = target.Parse("a = a + (b = b * (c = c - (d = d / (e = e % 3))))", parameters);

			Assert.AreEqual(a, lambda.Invoke(args));
			Assert.AreEqual(a, args[0]);
			Assert.AreEqual(b, args[1]);
			Assert.AreEqual(c, args[2]);
			Assert.AreEqual(d, args[3]);
			Assert.AreEqual(e, args[4]);
		}

If we had a reference to the actual delegate and performed a dynamic invoke on it with the args array, the Asserts would succeed.

[holdenmai]

I do have a fix for this ready local as it was blocking some of my testing of edge cases on collection initialization #250 , I would just like to confirm which path we want to go down.

Do we want to update the Invoke call or add an additional call that supports the ref parameters?

[davideicardi]

See also this comment: https://github.com/dynamicexpresso/DynamicExpresso/issues/251#issuecomment-1225648318

[holdenmai]

I agree from a security perspective with what you said.

In order for the underlying compiled Delegate to update the invoked objects, the parameter has to be declared as a ByRefType as follows typeof(int).MakeByRefType() So there is already a security layer being added from the part of the framework we are leveraging. We could also only do the final set on the output value if the related parameter is a ByRef type. Additionally this functionality can either be turned on as a setting (I know there are other settings to disable assign), or from a different Invoke call (like InvokeWithByRef).

Personally I think the cleanest option is to have a new Setting EnableByRefParameters that is off by default, and when it's enabled the ByRef still controls whether or not the ref/out parameter assignment can be accomplished.

I think there would also be some merit in allowing this functionality in some way since there is a unit test specifically set up to allow setting to a parameter (Can_assign_a_parameter)

[holdenmai]

Realized this is very similar to #31