-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Use IndexOfAnyValues in Uri #78666
Use IndexOfAnyValues in Uri #78666
Conversation
Tagging subscribers to this area: @dotnet/ncl Issue DetailsContributes to #78204 There are more places we can use
|
// | ||
|
||
internal static unsafe bool IsValid(char* name, int pos, ref int returnedEnd, ref bool notCanonical, bool notImplicitFile) | ||
public static bool IsValid(ReadOnlySpan<char> hostname, bool iri, bool notImplicitFile, out int length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The iri
parameter is always constant, so we could handle the differences with a generic, e.g.
private interface IIsValidImpl
{
static abstract int IndexOfInvalidOrDelimiterChar(ReadOnlySpan<char> chars);
static abstract bool CheckFirstLabelCharacter(char c);
static abstract int DotIndex(ReadOnlySpan<char> chars);
static abstract int CalculateLabelLength(ReadOnlySpan<char> label);
}
but it seemed like it was adding more code than it's worth (although arguably more readable?).
{ | ||
if (char.IsAsciiLetterUpper(str[i])) | ||
Debug.Assert(!str.AsSpan(start, index).Contains(':'), | ||
"A colon should appear at most once, and must never be followed by letters."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ParseCanonicalName is handed already sanitized data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
Contributes to #78204
There are more places we can use
IndexOfAnyValues
inSystem.Uri
. I'll address those in future PRs to keep them reviewable, this is complex enough as it is.http://aaaaaaaa(...)
[50]http://aaaaaaaa(...)
[50]http://www.Micr(...)
[28]http://www.Micr(...)
[28]http://www.micr(...)
[28]http://www.micr(...)
[28]Benchmark code