lunarmodules / luacov

LuaCov is a simple coverage analyzer for Lua code.
http://lunarmodules.github.io/luacov/
MIT License
300 stars 68 forks source link

Improve line exclusion #27

Closed mpeterv closed 9 years ago

mpeterv commented 9 years ago

Some questions:

  1. Is there a point in always filtering some lines? Maybe if a line is hit, it makes sense to always report it?
  2. Zero hit exclusions return <ARGS>*function(<ARGS>) and <ARGS>*function(<ARGS>) are broken, they expand into patterns containing [...]**, which typically never match. I think these exclusions are invalid anyway, because <ARGS> as expressions should always generate a trace.
  3. Comment for the first zero hit exclusion says "var1 var2," multi columns table stuff, what does that mean? var1 var2, can rarely be valid syntax.
hishamhm commented 9 years ago

Is there a point in always filtering some lines? Maybe if a line is hit, it makes sense to always report it?

I don't remember the rationale exactly as it's been a long time, but I think it's because some lines of this kind were reported as hits under Lua 5.1 and not reported under Lua 5.2 (or vice-versa), and this would skew the percentages when comparing coverage among both different versions.

Zero hit exclusions return _function() and _function() are broken, they expand into patterns containing [...]**, which typically never match.

Yes, that seems right -- the "*" are spurious there and should be removed

I think these exclusions are invalid anyway, because as expressions should always generate a trace.

They should, but if we added those lines it was probably because the debug hook was not reporting hits at those lines (I think they report a hit inside the function body).

Comment for the first zero hit exclusion says "var1 var2," multi columns table stuff, what does that mean? var1 var2, can rarely be valid syntax.

This was probably meant to be "var1, var2,"

mpeterv commented 9 years ago

I think these exclusions are invalid anyway, because as expressions should always generate a trace.

They should, but if we added those lines it was probably because the debug hook was not reporting hits at those lines (I think they report a hit inside the function body).

Opcodes for constants and locals are bound to line of the next token, and that token can't be on another line because of function token, so I'm not sure how that's possible. Simple test:

luac -l - <<< 'local a = 3
return a  
, nil
, 4
, "foo"
, function()
end'
main <stdin:0,0> (8 instructions at 0x9793df0)
0+ params, 6 slots, 1 upvalue, 1 local, 3 constants, 1 function
    1   [1] LOADK       0 -1    ; 3
    2   [3] MOVE        1 0
    3   [4] LOADNIL     2 0
    4   [5] LOADK       3 -2    ; 4
    5   [6] LOADK       4 -3    ; "foo"
    6   [7] CLOSURE     5 0 ; 0x9794120
    7   [7] RETURN      1 6
    8   [7] RETURN      0 1
hishamhm commented 9 years ago

Simple test:

As I said, I really don't remember the specifics, but I do remember that debug line hooks behave differently in Lua 5.1 vs 5.2+ (possibly vs LuaJIT too)

mpeterv commented 9 years ago

The test runs the same on Lua 5.1, 5.2 and 5.3 for me, can't check LuaJIT. I'll try to do that, also perhaps trace the addition of this rule. Either way, the rule is broken, and I'll open another PR if I find any clues.

hishamhm commented 9 years ago

Ok, merging this. Thank you!!

mpeterv commented 9 years ago

Thanks!