corvus-dotnet / Corvus.Globbing

A zero allocation globbing library
Apache License 2.0
18 stars 1 forks source link

GlobTokenizer.Tokenize should take Span<GlobToken> as in, not ref #2

Open idg10 opened 2 years ago

idg10 commented 2 years ago

Currently we have this signature:

public static int Tokenize(in ReadOnlySpan<char> glob, ref Span<GlobToken> tokenizedGlob)

I'm assuming the thinking with ref here is that the code will modify the tokenizedGlob, hence ref instead of in. But this is thinking at the wrong level of indirection.

The ref here actually gives Tokenize the capability of modifying the variable that the caller supplies as tokenizedGlob so that it refers to an entirely different Span<GlobToken>. And this method has no need of that.

If we change ref to in, we state, correctly, that this method will not attempt to modify which memory the caller's variable refers to. But we retain the ability to modify what's in that memory, because we've got a Span, and not a ReadOnlySpan.