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

Add support for shimming operators #31

Closed
8 tasks done
Miista opened this issue Jan 28, 2024 · 4 comments · Fixed by #33
Closed
8 tasks done

Add support for shimming operators #31

Miista opened this issue Jan 28, 2024 · 4 comments · Fixed by #33
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed priority: medium The issue has medium priority
Milestone

Comments

@Miista
Copy link
Owner

Miista commented Jan 28, 2024

Implicit vs. explicit operators.

Confirm by trying it out in the sandbox.

Currently not supported. Will be added in this issue and linked PR #33

Tasks

  • Implement support for conversion operators (both implicit and explicit)
  • Implement support for arithmetic operators
  • Implement support for equality operators
  • Implement support for comparison operators

Tests

For all of the following operators we need:

  1. Shim tests
  2. Exercise tests
  • Conversion operators
  • Arithmetic operators
  • Equality operators
  • Comparison operators

The list of operators (and their names) is taken from here: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/operator-overloading#overloadable-operators

Operators to support

Arithmetic

  • +x
  • -x
  • !x
  • ~x
  • ++
  • --
  • true I cannot find a good way to express this operation in an expression tree.
  • false I cannot find a good way to express this operation in an expression tree.
  • x + y
  • x - y
  • x / y
  • x % y
  • x & y
  • x | y
  • x ^ y
  • x << y
  • x >> y
  • x >>> y Expression trees cannot contain this operator. This is a limitation on the part of the compiler.

Equality

  • x == y
  • x != y

Comparison

  • x < y
  • x > y
  • x <= y
  • x >= y
@Miista Miista added the question/investigation Further information or investigation is required label Jan 28, 2024
@Miista
Copy link
Owner Author

Miista commented Jan 28, 2024

I can confirm that we do not currently support this.

Given the following MVE:

public class OverridenOperatorClass
{
    public static explicit operator bool(OverridenOperatorClass c) => false;
}

public static void Main(string[] args)
{
    Shim
        .Replace(() => (bool) Is.A<OverridenOperatorClass>())
        .With(delegate { return true; });
}

This throws the following exception:

Unhandled Exception: System.NotImplementedException: Unsupported expression
   at Pose.Helpers.ShimHelper.GetMethodFromExpression(Expression expression, Boolean setter, Object& instanceOrType) in C:\Rider\pose\src\Pose\Helpers\ShimHelper.cs:line 42
   at Pose.Shim.ReplaceImpl[T](Expression`1 expression, Boolean setter) in C:\Rider\pose\src\Pose\Shim.cs:line 68
   at Pose.Shim.Replace[T](Expression`1 expression, Boolean setter) in C:\Rider\pose\src\Pose\Shim.cs:line 62
   at Pose.Sandbox.Program.Main(String[] args) in C:\Rider\pose\src\Sandbox\Program.cs:line 36

This seems to be caused by the expression type not being supported. Specifically, ExpressionType.Convert is unsupported.

@Miista Miista self-assigned this Jan 28, 2024
@Miista Miista added this to the v2.2 milestone Jan 28, 2024
@Miista Miista changed the title Do we support shimming overloaded operators? Add support for shimming operators Jan 28, 2024
@Miista Miista added enhancement New feature or request priority: medium The issue has medium priority and removed question/investigation Further information or investigation is required labels Jan 28, 2024
Miista pushed a commit that referenced this issue Jan 28, 2024
Please note, though, that the shim cannot be applied to a specific instance as the operator is static.
@Miista
Copy link
Owner Author

Miista commented Jan 28, 2024

I've added support for shimming conversion operators. Please note, though, that the shim cannot be applied to a specific instance as the operator is static.

Other operators e.g. + remains to be added.

The modus operandi so far has been to:

  1. Add the test for a given operator (they are all much the same)
  2. Add support once the test fails

I'm encountering some issues with the following operators:

  • Increment: The expression tree (which is used for the call to Replace) cannot contain an assignment expression. The increment operator (both when used as prefix and postfix) is exactly an assignment operator.
  • true/false: How to make an expression which can be used to extract the true/false operator method?

Aside from the above operator shimming should be implemented now. Tests (both positive and negative) remain to be implemented for all operators.

@Miista Miista linked a pull request Jan 28, 2024 that will close this issue
Miista pushed a commit that referenced this issue Jan 28, 2024
Please note, though, that the shim cannot be applied to a specific instance as the operator is static.
@Miista Miista added the help wanted Extra attention is needed label Jan 28, 2024
Miista pushed a commit that referenced this issue Jan 28, 2024
We now support:
* Comparison
* Equality
* Most of arithmetic
Miista pushed a commit that referenced this issue Jan 28, 2024
We now support:
* +
* ++
* -
* --
* ~
* true
* false
@Miista
Copy link
Owner Author

Miista commented Mar 5, 2024

How to handle double casting? That is, assume the following code:

class OperatorsClass
{
    public static implicit operator double(OperatorsClass c) => double.MinValue;
}

var instance = new OperatorsClass();
var doubleCasted = (long)(int)instance; // Double-cast

Attempting to get the method from the expression (long)(int)instance will yield Convert(Convert(...)) from which we will fail to get a method.

Miista pushed a commit that referenced this issue Mar 5, 2024
Miista pushed a commit that referenced this issue Apr 11, 2024
Miista pushed a commit that referenced this issue Apr 11, 2024
@Miista
Copy link
Owner Author

Miista commented Apr 11, 2024

Operators shimming is now implemented and the README has been updated. I did discover that when attempting to shim an operator for which the type has no overload (e.g. 1 + 1) the error message is rather cryptic. I will look into making it more clear what went wrong.

UPDATE 2024-04-12: I have added a better error message for the above scenario.

Miista added a commit that referenced this issue May 2, 2024
Please note, though, that the shim cannot be applied to a specific instance as the operator is static.
Miista added a commit that referenced this issue May 2, 2024
We now support:
* Comparison
* Equality
* Most of arithmetic
Miista added a commit that referenced this issue May 2, 2024
We now support:
* +
* ++
* -
* --
* ~
* true
* false
Miista added a commit that referenced this issue May 2, 2024
Miista added a commit that referenced this issue May 2, 2024
@Miista Miista modified the milestones: v2.2, v2.1 Jun 6, 2024
@Miista Miista closed this as completed in #33 Jun 6, 2024
Miista added a commit that referenced this issue Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed priority: medium The issue has medium priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant