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

Remove unnecessary checks in baseparser #112

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

naitoh
Copy link
Contributor

@naitoh naitoh commented Feb 15, 2024

Why

next_data = @source.buffer
if next_data.size < 2
@source.read
next_data = @source.buffer
end
if next_data[0] == ?<
if next_data[1] == ?/
@nsstack.shift
last_tag = @tags.pop
md = @source.match( CLOSE_MATCH, true )
if md and !last_tag
message = "Unexpected top-level end tag (got '#{md[1]}')"
raise REXML::ParseException.new(message, @source)
end
if md.nil? or last_tag != md[1]
message = "Missing end tag for '#{last_tag}'"
message << " (got '#{md[1]}')" if md
raise REXML::ParseException.new(message, @source)
end
return [ :end_element, last_tag ]
elsif next_data[1] == ?!
md = @source.match(/\A(\s*[^>]*>)/um)
#STDERR.puts "SOURCE BUFFER = #{source.buffer}, #{source.buffer.size}"
raise REXML::ParseException.new("Malformed node", @source) unless md
if md[0][2] == ?-
md = @source.match( COMMENT_PATTERN, true )
case md[1]
when /--/, /-\z/
raise REXML::ParseException.new("Malformed comment", @source)
end
return [ :comment, md[1] ] if md
else
md = @source.match( CDATA_PATTERN, true )
return [ :cdata, md[1] ] if md
end
raise REXML::ParseException.new( "Declarations can only occur "+
"in the doctype declaration.", @source)
elsif next_data[1] == ??
return process_instruction
else
# Get the next tag
md = @source.match(TAG_MATCH, true)
unless md
raise REXML::ParseException.new("malformed XML: missing tag start", @source)
end
tag = md[1]
@document_status = :in_element
prefixes = Set.new
prefixes << md[2] if md[2]
@nsstack.unshift(curr_ns=Set.new)
attributes, closed = parse_attributes(prefixes, curr_ns)
# Verify that all of the prefixes have been defined
for prefix in prefixes
unless @nsstack.find{|k| k.member?(prefix)}
raise UndefinedNamespaceException.new(prefix,@source,self)
end
end
if closed
@closed = tag
@nsstack.shift
else
@tags.push( tag )
end
return [ :start_element, tag, attributes ]
end
else
md = @source.match( TEXT_PATTERN, true )
text = md[1]
if md[0].length == 0
@source.match( /(\s+)/, true )
end

          next_data = @source.buffer
          if next_data.size < 2
            @source.read
            next_data = @source.buffer
          end
          if next_data[0] == ?<
              :
            (omit)
              :
          else # next_data is a string of one or more characters other than '<'.
            md = @source.match( TEXT_PATTERN, true ) # TEXT_PATTERN = /\A([^<]*)/um
            text = md[1]
            if md[0].length == 0 # md[0].length is greater than or equal to 1.
              @source.match( /(\s+)/, true )
            end

This is an unnecessary check because md[0].length is greater than or equal to 1.

[Why]
https://github.com/ruby/rexml/blob/444c9ce7449d3c5a75ae50087555ec73ae1963a8/lib/rexml/parsers/baseparser.rb#L352-L425
```
          next_data = @source.buffer
          if next_data.size < 2
            @source.read
            next_data = @source.buffer
          end
          if next_data[0] == ?<
              :
            (ommit)
              :
          else # next_data is a string of one or more characters other than '<'.
            md = @source.match( TEXT_PATTERN, true ) # TEXT_PATTERN = /\A([^<]*)/um
            text = md[1]
            if md[0].length == 0 # md[0].length is greater than or equal to 1.
              @source.match( /(\s+)/, true )
            end
```
This is an unnecessary check because md[0].length is greater than or equal to 1.
@kou kou merged commit fc6cad5 into ruby:master Feb 15, 2024
39 checks passed
@kou
Copy link
Member

kou commented Feb 15, 2024

Good catch!

@naitoh naitoh deleted the remove_unnecessary_check_in_baseparser branch February 15, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants