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

Exception when using .? operator for nullable fields #169

Closed
ealeykin opened this issue Oct 19, 2021 · 5 comments · Fixed by #171
Closed

Exception when using .? operator for nullable fields #169

ealeykin opened this issue Oct 19, 2021 · 5 comments · Fixed by #171
Labels

Comments

@ealeykin
Copy link
Contributor

Looks like the following will try to create Nullable for Nullable.

    class Scope
    {
        public int? Value { get; set; }
    }
    
    class Program
    {
        static void Main(string[] args)
        {

            var interpreter = new Interpreter();
            var lambda = interpreter.Parse("Scope?.Value", new Parameter("Scope", typeof(Scope)));
            var result = lambda.Invoke(new Scope());
            Console.WriteLine(result);
        }
    }

this will throw an exception

 System.ArgumentException: GenericArguments[0], 'System.Nullable`1[System.Int32]', on 'System.Nullable`1[T]' violates the constraint of type 'T'.

@ealeykin
Copy link
Contributor Author

Expression expression2 = this.ParseMemberAccess((Type) null, expression1);

if (expression2.Type.IsValueType)
  expression2 = Parser.PromoteExpression(expression2, typeof (Nullable<>).MakeGenericType(expression2.Type), true);

This check - "expression2.Type.IsValueType" will return true for int? and an additional check like "Nullable.GetUnderlyingType(expression2.Type) is null" should be added to if statement.

@ealeykin ealeykin mentioned this issue Oct 19, 2021
metoule added a commit to metoule/DynamicExpresso that referenced this issue Oct 19, 2021
…on that ensures a nullable type isn't converted to a nullable of nullable.

Fixes dynamicexpresso#169
@metoule
Copy link
Contributor

metoule commented Oct 19, 2021

Oh I didn't see your pull request. There's actually an existing method that does the check already: GenerateNullableTypeConversion.

@ealeykin
Copy link
Contributor Author

ealeykin commented Oct 19, 2021

@davideicardi @metoule Found one more unclear behaviour

new Interpreter().SetVariable("x", (int?)null).Eval("x?.ToString()");

This will throw

DynamicExpresso.Exceptions.ParseException: Invalid Operation (at index 13).
 ---> System.InvalidOperationException: The binary operator Equal is not defined for the types 'System.Nullable`1[System.Int32]' and 'System.Object'.

@metoule
Copy link
Contributor

metoule commented Oct 19, 2021

That exception occurs because of the internal conversion that happens under the hood:

x?.ToString()

is converted to:

int? x;
object nullConstant = null;
if (x == nullConstant) {  // invalid operation
    return nullConstant;
} else {
   return x.ToString();
}

but a nullable integer can't be compared to an object.

Fortunately, it's possible to use the CheckAndPromoteOperands method to mitigate this issue. I've updated my pull request.

@davideicardi
Copy link
Member

Thanks @halamah and @metoule for bug report and fix! New version is on the way ...

davideicardi pushed a commit that referenced this issue Oct 21, 2021
* ?. and ?[ operators now use the existing GenerateNullableTypeConversion that ensures a nullable type isn't converted to a nullable of nullable.
Fixes #169

* Promote operands before generating the equality check on ?. and ?[ operators.
davideicardi pushed a commit that referenced this issue Oct 27, 2021
* ?. and ?[ operators now use the existing GenerateNullableTypeConversion that ensures a nullable type isn't converted to a nullable of nullable.
Fixes #169

* Promote operands before generating the equality check on ?. and ?[ operators.

* No longer consider ?. and ?[ as tokens, because it prevents the proper parsing of an array of a nullable type (e.g. int?[]).

* Improve type parsing to support arrays and generics.
Fixes #172

* No longer consider ?. and ?[ as tokens, because it prevents the proper parsing of an array of a nullable type (e.g. int?[]).

* Improve type parsing to support arrays and generics.
Fixes #172
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants