Conversation
Coverage counted the statement holding a heredoc twice per execution when
the heredoc's interpolation sat on a later line, and marked that
interpolation's line as code although nothing ever counted there. MRI
reports the statement once and leaves the line nil. Two fixes:
- A statement that is a call re-emitted its coverage line event when it
built the call itself, if building its receiver or arguments had moved
the current line elsewhere (an interpolation on another line, statements
inside parentheses, ...). A statement's coverage event is now emitted
once, when the statement starts; the call site only refreshes the line
for backtraces.
- A lone statement inside #{} is not a line event of its own (the string it
is part of is), so the parser no longer leaves its line marked as code.
Several statements in one interpolation each remain a line event, as in
MRI.
Un-excludes test_eval from the MRI coverage tests.
| // A lone statement in an interpolation is not a line event of its own (MRI); the | ||
| // string it is part of is. Several statements in one interpolation each remain one. | ||
| $6.unsetNewline(); | ||
| p.uncoverLine(@6.start()); |
There was a problem hiding this comment.
I cannot find equivalent logic in CRuby for this; the lines related only unset newline and have no additional logic:
https://github.com/ruby/ruby/blob/b2ec82d4ea84f5526b3759923159a039bc5c1d3b/parse.y#L6088-L6089
I'd like to understand why we need this and CRuby does not appear to do the same.
There was a problem hiding this comment.
CRuby gets the same result from nd_unset_fl_newline alone because it decides which lines are coverable at compile time, from the newline flag. iseq_compile_each adds RUBY_EVENT_COVERAGE_LINE only to nodes that still have the flag:
https://github.com/ruby/ruby/blob/b2ec82d4ea84f5526b3759923159a039bc5c1d3b/compile.c#L11045-L11052
and iseq_set_sequence sets a line's entry from nil to 0 only when that event lands on an instruction:
https://github.com/ruby/ruby/blob/b2ec82d4ea84f5526b3759923159a039bc5c1d3b/compile.c#L2713-L2720
So once the parser clears the flag, the line was never coverable.
JRuby marks coverable lines earlier, in the parser. newline_node sets the flag and marks the line at the same time. It has to happen there because method bodies are built lazily, so the IR can't mark lines for methods that never run. By the time this action clears the flag, the line is already marked 0, and it would show up as code that never ran where CRuby shows nil. The extra call undoes what newline_node did.
Your question also led me to a bug in that call. It set the line back to nil without checking anything, which wiped out marks left by other statements on the same line. It now only undoes the mark the lone statement added itself, and only if nothing else had marked that line (db35bd1).
CRuby also counts a statement on the line of the first instruction after its line event, not on the line where the statement starts. For example, x =\n y.size counts on line 2, and x = <<~EOS counts on the line of its first interpolation. JRuby treated this differently in several ways, so I pushed a few more commits:
- dbf912c: a statement nested in another (such as one of several statements in
#{}) no longer replaces the outer statement's pending line event. - 30039a8: a call built before a statement's first instruction no longer moves the statement's count to an unmarked line. Most multi-line statements used to read
[1, 0, 0]. Backtrace lines don't change. - 814037b: multi-line array, hash, string, regexp and symbol literals get the line they start on.
- 92a5b70: a new
LineEventshelper finds each statement's first instruction. The parser,Coverage.line_stuband the IR builder all use it, so marked lines and counted lines agree. - a76bc29: the coverage array is no longer one line short when a heredoc ends the file.
|
Let's get this in with the other coverage work in 10.1.3.0. |
Setting the interpolation's line back to nil also wiped out a mark left by another statement on that line, such as a statement in a block earlier on the same line or the first of several statements in one interpolation. Undo only the mark the lone statement's newline_node added, and only when no earlier statement had already marked the line.
A statement's line event waits for the statement's first instruction. When
building its value reached a statement of its own first, such as one of
several statements in an interpolation, that statement's event replaced
the outer one, so coverage never counted the outer statement's line:
x = "#{\n a = 1\n a\n}" read [0, 1, 1, nil] where MRI reads [1, 1, 1, nil].
When coverage is on, emit the outer statement's pending event before the
inner one replaces it.
A begin statement now asks for its body's line, the line the parser marks
for it, so emitting its event does not count the begin keyword's line.
A statement's line event waits for its first instruction and then took the line of whatever call had been built since, so o.b =\n [].size counted on the [].size line, a line the parser never marked: the results read [1, 0, 0] instead of counting the statement. Emit the event on the line it was requested for, then restore the call's line for backtraces, so backtraces do not change. As in MRI, a statement is a line event unless the last line event was on the same line; calls built in between no longer hide one (the 1 in if\n [].size then 1 end was not counted). A pending coverage event is no longer replaced by a call's backtrace line either.
An array literal took the line of its first element, a hash literal the line of its first key, a string the line its contents end on (or, for "" "..." concatenated, the line of the second string), a regexp without interpolation the line its contents end on, and a dynamic symbol, regexp, or backtick string the start of its contents, which for an empty production is not a real position (nor is it for the elements of %W, %I and %i lists, or a "string": label). MRI reports each on the line of its opening token; use that line for all of them.
MRI counts a statement on the line of its first instruction, which is a
later line whenever evaluating the statement starts with a part on one:
x =\n [].size counts on line 2, and so does x = <<~EOS whose body starts
with an interpolation (x = <<~EOS\n #{1.to_s}\nEOS reads [nil, 1, nil]).
JRuby marked and counted the line the statement starts on.
LineEvents finds the node whose instructions come first, following
assignment values, call receivers, conditions, the first element of an
array or key of a hash that MRI does not build as one literal, the first
interpolation of a string that does not start with a literal part, and
so on. The parser marks its line, Coverage.line_stub reports it, and the
IR builder counts the statement on it. As in MRI, a statement inside
another that starts with the same instruction shares its line event,
and a statement is no line event when the last one started on its line.
Along the way:
- The newline flags the lexer puts on heredoc lines for dedenting are
cleared after dedenting, so the IR builder no longer takes those lines
for statements.
- An ensure body is built before the body it protects; its line events
no longer count as the last ones when building the protected body.
- A lone conditional in an interpolation stays a line event, as MRI
counts its branches.
Against MRI 3.4, Coverage.line_stub now matches for 749 of the 840
files in lib/ruby/stdlib (510 before), with no line that matched before
differing now.
After reading a heredoc the lexer goes back to the line the heredoc
started on, and only skips past the heredoc when it reads the next line.
When the heredoc ends the file there is no next line, so the coverage
array stopped short: x = <<~EOS\n #{1.to_s}\nEOS read [nil, 1] instead of
[nil, 1, nil]. Size it by the last line read instead.
Sounds good! I also have a branch for If we can ship all of that in 10.1.3.0, I think it will be the biggest change to JRuby’s Coverage support since it was introduced! |
An undef took the line of the first name it undefines, so with that name on the next line, line coverage counted undef\n a on the second line instead of the first, where MRI counts it.
MRI parses the arms of a ternary as statements, so each is a line event of its own: in x = y.size ?\n 1 : 2, the arm on the second line is counted by line coverage and reported to line tracing. JRuby parsed them as plain expressions, so that line read nil.
|
I pushed two more commits:
|
Coverage counted a statement holding a heredoc twice per execution when the heredoc's interpolation sat on a later line, and marked the interpolation's line as code although nothing ever counted there. A statement's coverage event is now emitted once, when the statement starts. The call site only refreshes the line for backtraces (
determineIfWeNeedLineNumberForCall). Also, a lone statement inside#{}is not a line event. The string it is part of is, so the parser no longer leaves that line marked as code (uncoverLine). Several statements in one interpolation each remain a line event.