ur5us has quit [Remote host closed the connection]
ur5us has joined #jruby
ur5us has quit [Read error: Connection reset by peer]
rusk has quit [Read error: Connection reset by peer]
rusk has joined #jruby
ur5us has joined #jruby
ur5us has quit [Ping timeout: 252 seconds]
ur5us has joined #jruby
ur5us has quit [Ping timeout: 265 seconds]
ur5us has joined #jruby
retro-cz[m] has joined #jruby
retro-cz[m] is now known as simi[m]
ur5us has quit [Ping timeout: 240 seconds]
nirvdrum has joined #jruby
<enebo[m]> lopex Can you accept this PR: https://github.com/jruby/jcodings/pull/32
<lopex> enebo[m]: sure
<enebo[m]> lopex: thanks buddy :)
lucasb has joined #jruby
xardion has quit [Remote host closed the connection]
xardion has joined #jruby
subbu is now known as subbu|away
lanceball has quit [Changing host]
lanceball has joined #jruby
joast has quit [Quit: Leaving.]
subbu|away is now known as subbu
joast has joined #jruby
<headius[m]> g'day!
<headius[m]> bit more RubyVM sneaking into stdlib
<headius[m]> trying to decipher what this is doing
<headius[m]> I guess this is trying to get a baseline array of `0` and `nil` by walking iseq looking for line events
<headius[m]> maybe this can be written simply with ripper
<headius[m]> enebo: I implemented it using NodeVisitor but like some othe tests the coverable lines don't match CRuby
<enebo[m]> this strikes me as some extra piece of data which should come from actually building source
<headius[m]> I agree, and we do build this array during parse but it's not easy to access
<headius[m]> and I don't know if it would be any more correct
<headius[m]> this basically emulates that array built in ParserConfiguration that had the bad scaling factor
<enebo[m]> well I meant as an impl detail since we essentially have access to the AST
<enebo[m]> This PR will not work against streamed code for instance
<enebo[m]> not that that is very common
<headius[m]> you have to give it a filename so I'm not sure that matters
<enebo[m]> heh ok well a whole in their API :)
<headius[m]> I guess this is so you can get a completely uncovered base list of counts for a given file without loading it or having coverage one
<headius[m]> on
<headius[m]> so if we could do a parse that also exposes this coverage data, without coverage being on, it would be fine too
<enebo[m]> I don't have any idea why there is a difference if we are looking for newline nodes. I can only guess but it might not matter either if we output matching line events
<headius[m]> well for example that first missing coverable line comes from another multi-line construct:
<headius[m]> so it's counting the first line as a newline but not the second
<enebo[m]> hmm
<headius[m]> this smells a little like the multi-line hash and array coverage thing
<enebo[m]> yeah until I look into it then it is a mystery
<enebo[m]> I am going to try and continue on isourceposition removal
<enebo[m]> and then that when issue for 2.6
<enebo[m]> that second issue could change newlines for all I know
<headius[m]> I can probably merge this PR as is, especially if you think newline? ought to be correct but just isn't right now
<enebo[m]> we definitely are different in how we handle some stuff
<enebo[m]> well in spirit all iseq walking is doing it looking for newlines
<headius[m]> i.e. if this looks like the right impl but the parser is to blame then I might as well get this partial support in for now
<headius[m]> right
<enebo[m]> our equiv or near equiv would be IR built data but without doing even more work ASt is probably ok and should actually be different
<headius[m]> yeah IR is harder to walk also
<enebo[m]> not be different
<enebo[m]> startup interp should just be a list
<headius[m]> these newlines feed directly into the coverage instructions, so yeah
<headius[m]> I don't think much is gained by proceeding to compile and walk iseq
<enebo[m]> but those instrs are sourced from the AST so it makes snese to me to just use the faster thing
<enebo[m]> we may find out later there is a good reason for that in MRI but lacking any knowledge who knows
<enebo[m]> hah maybe this is why they added experimental AST
<enebo[m]> ripper really feels like the way all of this should have been done though
<headius[m]> well I can merge this and maybe this danmayer person can confirm how far off we are for his stuff
<enebo[m]> I mean you hit line 5 and any things marked on line 5 is covered
<headius[m]> apparently he's got us running green in CI now that `oneshot_lines` has landed
<enebo[m]> I guess though that even ripper does not tell you the span
<enebo[m]> an array from 5-35 may just say it happened in 6 (which is weird) whereas I think we say 5
<headius[m]> yeah
<enebo[m]> 5 feels more right to me but in most cases the start [ and the first element are on the first line
<headius[m]> ok I don't get it...
<enebo[m]> HAHAH
<enebo[m]> leave out the a =
<enebo[m]> it would doubly amusing if the second line issue has to do with eval'ing code
<headius[m]> Ladies and gentlemen... I give you Ruby
<enebo[m]> HAHAHA
<enebo[m]> that earned an extra A
<enebo[m]> yeah so I half wonder if {} is more complicated by potentially also being closures
<headius[m]> hahah
<headius[m]> this is even better
<enebo[m]> perhaps it is more of an impl artifact
<headius[m]> MRI stays the same
<enebo[m]> ah yeah that makes sense
<headius[m]> our result makes more sense
<enebo[m]> so we actually make a newline for any call if I recall
<enebo[m]> Although maybe I am over simplifying that
<enebo[m]> It could be the iseq advantage!
<enebo[m]> in IRBuilder I believe we do not emit the same line instr twice
<enebo[m]> That actually may be a clue to this...maybe I have an interesting comment in there
<enebo[m]> (note: that would be a different line for us)
<headius[m]> gimme a review explaining we'll look into the differences outside PR
<enebo[m]> also since we output call before hash we may line line 4, line 1 out of order too
<headius[m]> so there's obviously weird things afoot that are not fixable in this PR
<headius[m]> yeah I am just using actual node line numbers for this logic
<enebo[m]> yeah we maybe can eliminate some newlines where we couldn't before
<enebo[m]> in the past newline was used as an "insulating" node...so if you have a construct it would check a child and say do this if it is an arraynode or whatnot...but then they would newline wrap it and then it would not do that action
<enebo[m]> After they followed us in removing it as a full-fledged node I believe all that logic went away
<enebo[m]> The parser is complicated we maybe still have something in there adding extra newline field sets we do not need
<headius[m]> ah right
<enebo[m]> If you would have asked me last week I would have said who cares :)
<headius[m]> yeah well I was very surprised danmayer said we passed everything after he worked around missing `line_stub`
<headius[m]> maybe he has bad tests
<headius[m]> or maybe it doesn't matter that much
<enebo[m]> no it is likely we probably emit things off but still overlapping in some way with a single multiline statement
<enebo[m]> if they use parser gem it provides start/end line ranges and we hit something in that range
<enebo[m]> or at least this is my guess
<enebo[m]> return 0 or 1 in something from 0-3 will still say 0-3 was hit
<enebo[m]> without more knowledge it feels like a good guess :)
<enebo[m]> it is also how I would tackle coverage
<headius[m]> I have no explanation why hash is different
<enebo[m]> my very vague assertion above also feels good :)
<enebo[m]> {} can be hash OR a block
<enebo[m]> perhaps newline is not emitted until first thing in either
<enebo[m]> in a block first line would be the thing which mattered as well
<enebo[m]> but then all other lines as well
<enebo[m]> my main reason for guessing that is contents of hash or block are parsed before knowing it is one or the other
<enebo[m]> LALR and all that
<headius[m]> ok different topic quick... did that jcodings change get merged and released?
<enebo[m]> the one I asked lopex about this morning...I don't know if he did it or not yet
<headius[m]> ok
<enebo[m]> and I do not know enough about jcodings to know if it is correct so I did not do it
<lopex> oh I merged it
<headius[m]> ah ok so waiting on a review
<headius[m]> 👍
<headius[m]> but no release yet, ok
<headius[m]> ok next one
<headius[m]> enebo: you mentioned a branch here: https://github.com/jruby/jruby/issues/6184
<headius[m]> that will fix this
<headius[m]> what branch do you mean?
<headius[m]> is this the IRFor elimination
<enebo[m]> the one we landed after fosdem
<enebo[m]> it was working with for until after that merge and then for broke
<headius[m]> master is still broken
<enebo[m]> it is a known issue
<enebo[m]> we have talked about it as well
<headius[m]> ok
subbu is now known as subbu|lunch
<headius[m]> jcodings is released
subbu|lunch is now known as subbu
ur5us has joined #jruby
ur5us has quit [Remote host closed the connection]
ur5us has joined #jruby
nirvdrum has quit [Ping timeout: 240 seconds]
lucasb has quit [Quit: Connection closed for inactivity]