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

Review Design/Implementation Handling Ref-Like Types #170

Closed
JasonBock opened this issue Jun 28, 2022 · 6 comments
Closed

Review Design/Implementation Handling Ref-Like Types #170

JasonBock opened this issue Jun 28, 2022 · 6 comments

Comments

@JasonBock
Copy link
Owner

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
Copy link
Owner Author

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 JasonBock added this to the 7.1.0 milestone Nov 8, 2022
@JasonBock JasonBock modified the milestones: 7.1.1, 8.0.0 Jul 17, 2023
@JasonBock JasonBock removed this from the 7.2.0 milestone Nov 7, 2023
@JasonBock
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

JasonBock commented Jul 9, 2024

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
Copy link
Owner Author

JasonBock commented Jul 11, 2024

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 JasonBock added this to the 9.0.0 milestone Jul 16, 2024
@JasonBock
Copy link
Owner Author

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

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

1 participant