Skip to content

Commit

Permalink
Remove XmlConvert.StrEqual and use Span<char>.StartsWith() instead. (d…
Browse files Browse the repository at this point in the history 
…otnet#74955)

* Use Span.SequenceEqual instead if manual loop over char array.

* Remove StrEqual.
Replace with StartsWith as suggested.

* Remove not needed brackets + fix tests

* Use switch expression for "verison", "encoding" and "standalone"

* Use ranges + one more switch expression

* Remove StartsWith helper method from ParsingState

* Address range feedback

* Add test for XmlTextReader.ReadChars

* Fix test

Co-authored-by: Traian Zaprianov <[email protected]>
  • Loading branch information
@TrayanZapryanov
TrayanZapryanov and TrayanZapryanov authored Oct 7, 2022
1 parent d10cc5a commit abffaf8
Show file tree
Hide file tree
Showing 5 changed files with 315 additions and 88 deletions.
75 changes: 37 additions & 38 deletions src/libraries/System.Private.Xml/src/System/Xml/Core/XmlTextReaderImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3643,7 +3643,7 @@ private bool ParseXmlDeclaration(bool isTextDecl)
}
}

if (!XmlConvert.StrEqual(_ps.chars, _ps.charPos, 5, XmlDeclarationBeginning) ||
if (!_ps.chars.AsSpan(_ps.charPos).StartsWith(XmlDeclarationBeginning) ||
XmlCharType.IsNameSingleChar(_ps.chars![_ps.charPos + 5]))
{
goto NoXmlDecl;
Expand Down Expand Up @@ -3741,10 +3741,10 @@ private bool ParseXmlDeclaration(bool isTextDecl)
int nameEndPos = ParseName();

NodeData? attr = null;
switch (_ps.chars[_ps.charPos])
switch (_ps.chars.AsSpan(_ps.charPos, nameEndPos - _ps.charPos))
{
case 'v':
if (XmlConvert.StrEqual(_ps.chars, _ps.charPos, nameEndPos - _ps.charPos, "version") && xmlDeclState == 0)
case "version":
if (xmlDeclState == 0)
{
if (!isTextDecl)
{
Expand All @@ -3753,9 +3753,8 @@ private bool ParseXmlDeclaration(bool isTextDecl)
break;
}
goto default;
case 'e':
if (XmlConvert.StrEqual(_ps.chars, _ps.charPos, nameEndPos - _ps.charPos, "encoding") &&
(xmlDeclState == 1 || (isTextDecl && xmlDeclState == 0)))
case "encoding":
if (xmlDeclState == 1 || (isTextDecl && xmlDeclState == 0))
{
if (!isTextDecl)
{
Expand All @@ -3765,9 +3764,8 @@ private bool ParseXmlDeclaration(bool isTextDecl)
break;
}
goto default;
case 's':
if (XmlConvert.StrEqual(_ps.chars, _ps.charPos, nameEndPos - _ps.charPos, "standalone") &&
(xmlDeclState == 1 || xmlDeclState == 2) && !isTextDecl)
case "standalone":
if ((xmlDeclState == 1 || xmlDeclState == 2) && !isTextDecl)
{
attr = AddAttributeNoChecks("standalone", 1);
xmlDeclState = 2;
Expand Down Expand Up @@ -3835,7 +3833,7 @@ private bool ParseXmlDeclaration(bool isTextDecl)
// version
case 0:
// VersionNum ::= '1.0' (XML Fourth Edition and earlier)
if (XmlConvert.StrEqual(_ps.chars, _ps.charPos, pos - _ps.charPos, "1.0"))
if (_ps.chars.AsSpan(_ps.charPos).StartsWith("1.0"))
{
if (!isTextDecl)
{
Expand All @@ -3861,18 +3859,18 @@ private bool ParseXmlDeclaration(bool isTextDecl)
xmlDeclState = 2;
break;
case 2:
if (XmlConvert.StrEqual(_ps.chars, _ps.charPos, pos - _ps.charPos, "yes"))
{
_standalone = true;
}
else if (XmlConvert.StrEqual(_ps.chars, _ps.charPos, pos - _ps.charPos, "no"))
{
_standalone = false;
}
else
switch (_ps.chars.AsSpan(_ps.charPos, pos - _ps.charPos))
{
Debug.Assert(!isTextDecl);
Throw(SR.Xml_InvalidXmlDecl, _ps.LineNo, _ps.LinePos - 1);
case "yes":
_standalone = true;
break;
case "no":
_standalone = false;
break;
default:
Debug.Assert(!isTextDecl);
Throw(SR.Xml_InvalidXmlDecl, _ps.LineNo, _ps.LinePos - 1);
break;
}
if (!isTextDecl)
{
Expand Down Expand Up @@ -3997,7 +3995,7 @@ private bool ParseDocumentContent()
{
goto ReadData;
}
if (XmlConvert.StrEqual(chars, pos, 6, "CDATA["))
if (chars.AsSpan(pos).StartsWith("CDATA["))
{
_ps.charPos = pos + 6;
ParseCData();
Expand Down Expand Up @@ -4217,7 +4215,7 @@ private bool ParseElementContent()
{
goto ReadData;
}
if (XmlConvert.StrEqual(chars, pos, 6, "CDATA["))
if (chars.AsSpan(pos).StartsWith("CDATA["))
{
_ps.charPos = pos + 6;
ParseCData();
Expand Down Expand Up @@ -4424,7 +4422,7 @@ private void ParseElement()
{
int startPos = _ps.charPos;
int prefixLen = colonPos - startPos;
if (prefixLen == _lastPrefix.Length && XmlConvert.StrEqual(chars, startPos, prefixLen, _lastPrefix))
if (prefixLen == _lastPrefix.Length && chars.AsSpan(startPos).StartsWith(_lastPrefix))
{
_curNode.SetNamedNode(XmlNodeType.Element,
_nameTable.Add(chars, colonPos + 1, pos - colonPos - 1),
Expand Down Expand Up @@ -4600,7 +4598,7 @@ private void ParseEndElement()
char[] chars = _ps.chars;
if (startTagNode.prefix.Length == 0)
{
if (!XmlConvert.StrEqual(chars, _ps.charPos, locLen, startTagNode.localName))
if (!chars.AsSpan(_ps.charPos).StartsWith(startTagNode.localName))
{
ThrowTagMismatch(startTagNode);
}
Expand All @@ -4609,9 +4607,9 @@ private void ParseEndElement()
else
{
int colonPos = _ps.charPos + prefLen;
if (!XmlConvert.StrEqual(chars, _ps.charPos, prefLen, startTagNode.prefix) ||
if (!chars.AsSpan(_ps.charPos).StartsWith(startTagNode.prefix) ||
chars[colonPos] != ':' ||
!XmlConvert.StrEqual(chars, colonPos + 1, locLen, startTagNode.localName))
!chars.AsSpan(colonPos + 1).StartsWith(startTagNode.localName))
{
ThrowTagMismatch(startTagNode);
}
Expand Down Expand Up @@ -6732,7 +6730,7 @@ private bool ParseDoctypeDecl()
}
}

if (!XmlConvert.StrEqual(_ps.chars, _ps.charPos, 7, "DOCTYPE"))
if (!_ps.chars.AsSpan(_ps.charPos).StartsWith("DOCTYPE"))
{
ThrowUnexpectedToken((!_rootElementParsed && _dtdInfo == null) ? "DOCTYPE" : "<!--");
}
Expand Down Expand Up @@ -6814,7 +6812,7 @@ private void SkipDtd()
}
}
// check 'PUBLIC'
if (!XmlConvert.StrEqual(_ps.chars, _ps.charPos, 6, "PUBLIC"))
if (!_ps.chars.AsSpan(_ps.charPos).StartsWith("PUBLIC"))
{
ThrowUnexpectedToken("PUBLIC");
}
Expand Down Expand Up @@ -6851,7 +6849,7 @@ private void SkipDtd()
}
}
// check 'SYSTEM'
if (!XmlConvert.StrEqual(_ps.chars, _ps.charPos, 6, "SYSTEM"))
if (!_ps.chars.AsSpan(_ps.charPos).StartsWith("SYSTEM"))
{
ThrowUnexpectedToken("SYSTEM");
}
Expand Down Expand Up @@ -7725,7 +7723,7 @@ private NodeData AddAttribute(int endNamePos, int colonPos)
_attrNeedNamespaceLookup = true;
int startPos = _ps.charPos;
int prefixLen = colonPos - startPos;
if (prefixLen == _lastPrefix.Length && XmlConvert.StrEqual(_ps.chars, startPos, prefixLen, _lastPrefix))
if (prefixLen == _lastPrefix.Length && _ps.chars.AsSpan(_ps.charPos).StartsWith(_lastPrefix))
{
return AddAttribute(_nameTable.Add(_ps.chars, colonPos + 1, endNamePos - colonPos - 1),
_lastPrefix,
Expand Down Expand Up @@ -8274,23 +8272,23 @@ private int IncrementalRead()
case IncrementalReadState.PI:
if (ParsePIValue(out startPos, out pos))
{
Debug.Assert(XmlConvert.StrEqual(_ps.chars, _ps.charPos - 2, 2, "?>"));
Debug.Assert(_ps.chars.AsSpan(_ps.charPos - 2).StartsWith("?>"));
_ps.charPos -= 2;
_incReadState = IncrementalReadState.Text;
}
goto Append;
case IncrementalReadState.Comment:
if (ParseCDataOrComment(XmlNodeType.Comment, out startPos, out pos))
{
Debug.Assert(XmlConvert.StrEqual(_ps.chars, _ps.charPos - 3, 3, "-->"));
Debug.Assert(_ps.chars.AsSpan(_ps.charPos - 3).StartsWith("-->"));
_ps.charPos -= 3;
_incReadState = IncrementalReadState.Text;
}
goto Append;
case IncrementalReadState.CDATA:
if (ParseCDataOrComment(XmlNodeType.CDATA, out startPos, out pos))
{
Debug.Assert(XmlConvert.StrEqual(_ps.chars, _ps.charPos - 3, 3, "]]>"));
Debug.Assert(_ps.chars.AsSpan(_ps.charPos - 3).StartsWith("]]>"));
_ps.charPos -= 3;
_incReadState = IncrementalReadState.Text;
}
Expand Down Expand Up @@ -8410,7 +8408,7 @@ private int IncrementalRead()
{
goto ReadData;
}
if (XmlConvert.StrEqual(chars, pos + 2, 7, "[CDATA["))
if (chars.AsSpan(pos + 2).StartsWith("[CDATA["))
{
pos += 9;
_incReadState = IncrementalReadState.CDATA;
Expand All @@ -8429,7 +8427,8 @@ private int IncrementalRead()

// ParseQName can flush the buffer, so we need to update the startPos, pos and chars after calling it
int endPos = ParseQName(true, 2, out _);
if (XmlConvert.StrEqual(chars, _ps.charPos + 2, endPos - _ps.charPos - 2, _curNode.GetNameWPrefix(_nameTable)) &&
string checkName = _curNode.GetNameWPrefix(_nameTable);
if ((endPos - _ps.charPos - 2) == checkName.Length && chars.AsSpan(_ps.charPos + 2).StartsWith(checkName) &&
(_ps.chars[endPos] == '>' || XmlCharType.IsWhiteSpace(_ps.chars[endPos])))
{
if (--_incReadDepth > 0)
Expand Down Expand Up @@ -8468,7 +8467,7 @@ private int IncrementalRead()

// ParseQName can flush the buffer, so we need to update the startPos, pos and chars after calling it
int endPos = ParseQName(true, 1, out _);
if (XmlConvert.StrEqual(_ps.chars, _ps.charPos + 1, endPos - _ps.charPos - 1, _curNode.localName) &&
if (endPos - _ps.charPos - 1 == _curNode.localName.Length && _ps.chars.AsSpan(_ps.charPos + 1).StartsWith(_curNode.localName) &&
(_ps.chars[endPos] == '>' || _ps.chars[endPos] == '/' || XmlCharType.IsWhiteSpace(_ps.chars[endPos])))
{
_incReadDepth++;
Expand Down
Loading

0 comments on commit abffaf8

Please sign in to comment.