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

Error clarity: Cannot pass stack allocated spans to instance methods on ref struct #23433

Closed
KrzysztofCwalina opened this issue Nov 28, 2017 · 12 comments
Labels
Area-Compilers Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. Language-C# Resolution-By Design The behavior reported in the issue matches the current design
Milestone

Comments

@KrzysztofCwalina
Copy link
Member

KrzysztofCwalina commented Nov 28, 2017

Version Used:
7.2

Steps to Reproduce:

class Program
{
    static void Main()
    {
        Span<byte> bytes = stackalloc byte[10];
        var foo = new Foo();
        foo.Bar(bytes);
    }
}

ref struct Foo
{
    public void Bar(Span<byte> buffer) {
        Console.WriteLine(buffer.Length);
    }
}

Expected Behavior:
The program should compile fine (possibly with some modifications/annotations.

Actual Behavior:
Error CS8350 This combination of arguments to 'Foo.Bar(Span)' is disallowed because it may expose variables referenced by parameter 'buffer' outside of their declaration scope ConsoleApp5 C:\Users\Krzysztof\Documents\Visual Studio 2017\Projects\ConsoleApp5\ConsoleApp5\Program.cs 9 Active

@KrzysztofCwalina KrzysztofCwalina changed the title Cannot pass stackallocated span to methods on ref struct Cannot pass stack allocated spans to methods on ref struct Nov 28, 2017
@KrzysztofCwalina KrzysztofCwalina changed the title Cannot pass stack allocated spans to methods on ref struct Cannot pass stack allocated spans to instance methods on ref struct Nov 28, 2017
@jcouv
Copy link
Member

jcouv commented Nov 29, 2017

Tagging @VSadov

@jcouv
Copy link
Member

jcouv commented Nov 30, 2017

Ping @VSadov

@VSadov
Copy link
Member

VSadov commented Nov 30, 2017

This is ByDesign as we have discussed. Either the struct must be readonly or the method must be a static method taking both instance and argument byval, or the method must be an in extension method on the struct.

Otherwise nothing stops the method from doing this.SomePart = buffer and corrupting the returnable instance with unreturnable data.
Compiler prevents that from happening, but error could be be clearer.

@VSadov VSadov added Resolution-By Design The behavior reported in the issue matches the current design Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. and removed Resolution-By Design The behavior reported in the issue matches the current design labels Nov 30, 2017
@VSadov VSadov changed the title Cannot pass stack allocated spans to instance methods on ref struct Error clarity: Cannot pass stack allocated spans to instance methods on ref struct Nov 30, 2017
@VSadov VSadov added this to the 15.7 milestone Nov 30, 2017
@jcouv jcouv added Resolution-By Design The behavior reported in the issue matches the current design and removed Bug labels Nov 30, 2017
@KrzysztofCwalina
Copy link
Member Author

KrzysztofCwalina commented Nov 30, 2017

It would be nice if I could somehow indicate I won't store the buffer in a field, e.g.:

    public void Bar(Span<byte> buffer) readonly {
        Console.WriteLine(buffer.Length);
    }

@VSadov
Copy link
Member

VSadov commented Dec 1, 2017

Individually readonly members could help too. As long as one knows what is wrong.
A post-modifier ‘readonly’ might be one of possible syntaxes. It is a whole new feature though.

@KrzysztofCwalina
Copy link
Member Author

Another idea (from @pakrym) would be to simply allow calling such members as long as they are marked unsafe:

public unsafe void Bar(Span<byte> buffer) {
        Console.WriteLine(buffer.Length);
}

@jcouv
Copy link
Member

jcouv commented Dec 15, 2017

@KrzysztofCwalina Since this is currently by-design, I'll go ahead and close this issue.
Feel free to open a csharplang issue if there is a language proposal to discuss. Thanks

@jcouv jcouv closed this as completed Dec 15, 2017
@tmds
Copy link
Member

tmds commented Jan 3, 2018

I'm trying to implement a TryAppend method that takes a ReadOnlySpan<byte> and I'm hitting this issue.

    public unsafe ref struct SpanWriter
    {
        // private ...
        public bool TryAppend(ReadOnlySpan<byte> data)
        {
            // ...
        }
    }

I guess I can't workaround this since I don't want to make my struct readonly.

@KrzysztofCwalina
Copy link
Member Author

@tmd, I think you can workaround this by changing the method to a static method.

        public static bool TryAppend(SpanWriter writer, ReadOnlySpan<byte> data)
        {
            // ...
        }

@naine
Copy link

naine commented Nov 15, 2021

For the benefit of others who come across this:

Problem with the workaround above is that the method presumably has to modify a position field within the SpanWriter, or maybe replace the span it holds with a slice of the unwritten portion. So it either has to be a non-readonly instance method or take the instance byref, either way getting CS8350.

Another workaround, if you know the implementation will not save the span and only copy from it, is to trick the compiler by recreating it:

TryAppend(MemoryMarshal.CreateReadOnlySpan(ref MemoryMarshal.GetReference(span), span.Length)); // No CS8350

Obviously this is unsafe.

@darren-clark
Copy link

I ran into this trying to make an extension method on SequenceReader<byte> that would call both CopyTo and Advance for convenience. With stackalloc span. So I need to have the reader by ref, and it seems that it's "safe", but not sure if I can convince the compiler.

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented May 10, 2022

I bumped into this issue when doing prototyping work for dotnet/runtime#54410. It would be nice if we could somehow indicate that the buffer will never be stored in the ref struct, even if it means marking the method as unsafe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. Language-C# Resolution-By Design The behavior reported in the issue matches the current design
Projects
None yet
Development

No branches or pull requests

7 participants