Rocks icon indicating copy to clipboard operation
Rocks copied to clipboard

Review Design/Implementation Handling Ref-Like Types

Open JasonBock opened this issue 3 years ago • 3 comments

With #167, I thought I addressed ref-like types, and....the tests in RefStructTests show that things do work. But it feels inconsistent. Some tests require the return value to be returned through a custom delegate. Others don't do this.

I need to figure out what truly needs to be done with ref-like types in mocks and ensure things are consistent and correct.

JasonBock avatar Jun 28 '22 01:06 JasonBock

Related issue: the generated delegates used the hash code of the method's signature to differentiate methods that have the same name but different signatures. This changes every time a test is run that generates these delegates. This doesn't break the mock itself, but the code diffs in the test will fail every time as the number changes. May be worth to see if there is a way to create a name that is unique for that delegate that will generate a consistent, repeatable name.

JasonBock avatar Jun 28 '22 01:06 JasonBock

One thing that may be coming in the future is the ability to pass ref structs to generics. Details can be found here. It's unclear if it'll show up in C# 13, or 14, or....never (?), but it was mentioned in recent C# design notes. If this would go through, it may make this relatively trivial, but....in the meantime, having specialized types for Span<> and related Span<> types may be beneficial for now.

JasonBock avatar Dec 17 '23 16:12 JasonBock

Did some experimental work, seems like I can create some core types for Span<> and ReadOnlySpan<>. Here is the sharplab.io link. The types are this:

public delegate Span<T> SpanReturnValue<T>();

public delegate bool SpanArgumentEvalution<T>(Span<T> @value);

[Serializable]
public sealed class SpanArgument<T>
    : Argument
{
	private readonly SpanArgumentEvalution<T>? evaluation;
    private readonly ValidationState validation;

    internal SpanArgument() => this.validation = ValidationState.None;

    internal SpanArgument(SpanArgumentEvalution<T> @evaluation)
    {
        this.evaluation = @evaluation;
        this.validation = ValidationState.Evaluation;
    }

    public bool IsValid(Span<T> @value) =>
        this.validation switch
        {
            ValidationState.None => true,
            ValidationState.Evaluation => this.evaluation!(@value),
            _ => throw new global::System.NotSupportedException("Invalid validation state."),
        };
}

public delegate ReadOnlySpan<T> ReadOnlySpanReturnValue<T>();

public delegate bool ReadOnlySpanArgumentEvalution<T>(ReadOnlySpan<T> @value);

[Serializable]
public sealed class ReadOnlySpanArgument<T>
    : Argument
{
	private readonly ReadOnlySpanArgumentEvalution<T>? evaluation;
    private readonly ValidationState validation;

    internal ReadOnlySpanArgument() => this.validation = ValidationState.None;

    internal ReadOnlySpanArgument(ReadOnlySpanArgumentEvalution<T> @evaluation)
    {
        this.evaluation = @evaluation;
        this.validation = ValidationState.Evaluation;
    }

    public bool IsValid(Span<T> @value) =>
        this.validation switch
        {
            ValidationState.None => true,
            ValidationState.Evaluation => this.evaluation!(@value),
            _ => throw new global::System.NotSupportedException("Invalid validation state."),
        };
}

Also, need to remind myself to write a test with a ref struct that isn't one of the span types.

JasonBock avatar Dec 20 '23 23:12 JasonBock

FWIW it looks like allows ref struct is going to be in C# 13 (https://devblogs.microsoft.com/dotnet/csharp-13-explore-preview-features/?utm_source=dlvr.it&utm_medium=mastodon#allows-ref-struct), so I should be able to create "generic" versions of Argument and friends that can be used in ref struct circumstances. Maybe something like this:

var myStructArg = new RefStructArgument<MyStruct>();
myStructArg.IsValid(new MyStruct());

var spanIntArg = new RefStructArgument<Span<int>>();
spanIntArg.IsValid([1, 2, 3]);

public ref struct MyStruct { }

public delegate T RefStructReturnValue<T>()
    where T : allows ref struct;

public delegate bool RefStructArgumentEvaluation<T>(T @value)
    where T : allows ref struct;

[Serializable]
public sealed class RefStructArgument<T>
    : Argument
    where T : allows ref struct
{
    private readonly RefStructArgumentEvaluation<T>? evaluation;
    private readonly ValidationState validation;

    internal RefStructArgument() => this.validation = ValidationState.None;

    internal RefStructArgument(RefStructArgumentEvaluation<T> @evaluation)
    {
        this.evaluation = @evaluation;
        this.validation = ValidationState.Evaluation;
    }

    public bool IsValid(T @value) =>
        this.validation switch
        {
            ValidationState.None => true,
            ValidationState.Evaluation => this.evaluation!(@value),
            _ => throw new global::System.NotSupportedException("Invalid validation state."),
        };
}

Will definitely need to experiment with this first to ensure this could work before committing to this. Probably look at the gen-d code that handles a Span<int> and manually change the gen-d code to see what works and/or breaks.

JasonBock avatar Jul 09 '24 20:07 JasonBock

Actually....what I just realized is that all I need to do is add where T : allows ref struct to the existing Argument<T> and everything will work. Even Predicate<T> has been updated to allow ref structs, along with Func<TResult>, so this may be really easy to do. I won't have to generate new types or even create these separate "ref struct" types. Just adding the constraints will be enough. (And then delete a fair amount of code from Rocks that I no longer need :) )

JasonBock avatar Jul 11 '24 03:07 JasonBock

Remember to add a test in ParamsTests in Rocks.IntegrationTests for a params ReadOnlySpan<string> - this fix may make things easier to mock.

JasonBock avatar Jul 26 '24 12:07 JasonBock