<headius> good morning
<headius> enebo: could you review https://github.com/jruby/jruby/pull/6770
<enebo[m]> ok
<headius> there are a couple additional places on master relating to splitting or inlining or something, I will just fix those directly after merging
<enebo[m]> headius: yeah inlining is pretty non-working (well how we hook it up not the inlining code itself) so don't worry too much about it
<enebo[m]> I like this change since it removes looking a TC to something where we know it is set it and forget it. IRScope will have the right line whereas we are too depend on moving parts to trust TC
<headius> yeah this might get revisited if I try to fix the JIT, because it doesn't push a backtrace and does arity-checking outside the method body
<headius> so in that case the bad frame just disappears
<headius> but I could generate a varargs wrapper that uses the file and line like this does
<enebo[m]> I have been looking at prepend issues
<enebo[m]> woof
<headius> oh good, I have not had motivation to do it
<headius> any luck?
<enebo[m]> hahah I almost lost all mine already
<headius> yeah digging around in the class object hierarchy is pretty gnarly
<enebo[m]> well it is funny when I look at https://github.com/jruby/jruby/issues/6445
<enebo[m]> I realize donv is correct in wondering why X method is not called. It is fixed in 3.0
<headius> oh hell
<enebo[m]> So that had to have been a lookup bug in MRI that was fixed because logically it makes sense X would be called
<headius> so maybe we punt these another release
<enebo[m]> well one thing is certain...no one can depend on the havior in 6445 since it will change
<enebo[m]> So that particular one can definitely be punted since no one can portably use it in MRI
<enebo[m]> but here is where it is weird for me and the other issues may be part of this
<enebo[m]> I changed searchNormalSuperclass to return getNonIncluedClass() if the klazz is prepended
<enebo[m]> and not get the superclass from then non included class
<enebo[m]> This almost fixes the issue but now I get 'Stuff X\nSruff X\nStuff M'
<enebo[m]> Ok well that is unexpected
<enebo[m]> So I guess I am on a prepended method X in a module made for the specific prepend
<enebo[m]> Then I get nonincluded which is just the original module for X so it prints again
<headius> ah yeah
<enebo[m]> but then it does actually then find the M from there
<enebo[m]> but if I getNonIncluded.getSuperClass it won't find M
<headius> this is the 6445 example?
<enebo[m]> Actually I wonder now what I figured out :) If the second call ends up finding the X version of the mehtod again but after that it finds the M then I don't know whhich X is which
<enebo[m]> yeah donv original one
<enebo[m]> oh wait no
<enebo[m]> sorry yeah
<enebo[m]> he has two very very similar open issues on prepend
<enebo[m]> #6445 top reported snippet
<headius> ok
<enebo[m]> we print stuff X and die trying to find super
<headius> where do you change getNonIncludedClass
<enebo[m]> 2.7- will only print sutff M which is wrong
<enebo[m]> 3.0 does what you would logically think is the correct behavior
<headius> like show me a patch
<headius> when X gets prepended into M, M's contents should move to a prepend shim and X gets included just below that
<enebo[m]> ignore what I have in the isPrepended thing. I was just screwing around in the debugger
<headius> method search of M would start at the original M and then should go up to the included X and the prepend shim
<enebo[m]> yeah I figured we would get a shim module for the prepend
<headius> logically 3.0 behavior does make sense to me with this model
<enebo[m]> non-included is actually wrong I think because it would then go to actual module it is defined from
<enebo[m]> Or at least I thought that
<headius> so where this gets fuzzy for me is that I don't quite understand what happens with modules included in modules
<enebo[m]> I learn this stuff and then lose my knowledge :)
<enebo[m]> I guess ancestors may be our friend
<headius> what I really would like is a diagram of the internal structure of this particular hierarch
<enebo[m]> if (module.methodLocation == module) list.add(module.getDelegate().getNonIncludedClass());
<enebo[m]> HAHAH
<headius> edipo.federle: you might be better at debugging CRuby than me... maybe you can figure out what the actual class hierarchy looks like
<enebo[m]> yeah I was thinking damn...why can't this debugger display this in a nice box diagram
<headius> depending on how X looks when prepended into M I can see both examples
<enebo[m]> but for us we stack up a big list of all modules into a single list and this may also explain our ordering issues
<headius> maybe if we figure out how they fixed this it will inform a solution on our end
<enebo[m]> hahah ok crud
<enebo[m]> A.ancestors in that snippet on == [A, M, Object, Kernel, BasicObject]
<enebo[m]> on 3.0 it is [A, X, M, Object, Kernel, BasicObject]
<headius> jeremyevans has his fingers in this stuff on the 3.0 timeline
<headius> aha
<headius> so this gets back to my weak understanding of including modules with includes
<headius> I believe at one point, if you included a module X that itself included Y, the target class would get separate includes of X and Y
<headius> at that moment
<headius> so if X included Y later it would not be seen
<enebo[m]> M.prepend(X) is not visible to ancestor chain until 3.0 but logically you would think it would be
<headius> they may have changed how modules with included modules work at some point and we did not track it
<enebo[m]> oh because we copy at that point
<headius> like instead of copying all modules out of that one at include time, they may do a separate walk of the module and its own includes
<enebo[m]> or make the ship
<headius> which would pick up changes later on
<enebo[m]> SHIP IT SHIM
<headius> but this is all just supposition
<headius> we did it that way at one point but it didn't match behavior at the time
<enebo[m]> well if we shim and then process other modules as part of the shim process it is pretty easy to see how order would matter
<enebo[m]> but beyond figuring out this ordering we do actually find X when we call 'stuff' :)
<headius> look at gatherModules in RubyModule
<headius> it goes and mines out the modules that are in the original (including the original) and then does separate includes of each in doIncludeModule
<headius> doPrependModule does the same logic
<headius> but this has to be the general area where we differ
<headius> I am just not confident we even have the hierarchy wired right so I'm not sure what to fix
<headius> patching around a broken hierarchy is why we are here
<headius> is why = may be why
<enebo[m]> there seems to be 3 different prepend bugs and two are heavily related to ordering of when something happens
<headius> yes
<enebo[m]> the third one is what I am looking at but it also perhaps is or isn't
<enebo[m]> I imagine if we change order of include M to be later we get a different behavior so it definitely could be part of it
<headius> or move the prepend up
<enebo[m]> what my pet theory is when we prepend/include we will set up a set of shims to live in the hiearchy of what it is included/prepended into but then anything later added gets added to original modules but the shimmed ones never see it
<headius> I feel like the only way this could work is if they changed it to not copy all modules out at include time
<enebo[m]> RCLASS_SET_INCLUDER(VALUE iclass, VALUE klass)
<enebo[m]> RB_OBJ_WRITE(iclass, &RCLASS_INCLUDER(iclass), klass);
<enebo[m]> }
<enebo[m]> {
<enebo[m]> RCLASS_SET_INCLUDER(origin, RCLASS_INCLUDER(iclass->klass));
<enebo[m]> This is 3.0 but that sort of looks like the included module put into a particular hierarchy is being recorded by the original module it is made from
<enebo[m]> I don't ever recall seeing this macro/function though.
<headius> neither do I
<enebo[m]> Maybe this is just some refactoring
<enebo[m]> 2.6 is quite a bit different and the logic looks more like ours with include_modules_at
<headius> ah yeah that is the one I know
<headius> that takes modules from over here and puts them over there
<headius> blame that macro and maybe we will get some useful info
<enebo[m]> ok well so I learned something...gist coming
<enebo[m]> I am distracting with 3.0 stuff I think
<enebo[m]> So 3.0 is recording module using sites and updating them 2.7 is order dependent
<enebo[m]> This I simplified to just behavior of include so prepend issue we have may be different
<enebo[m]> but it explains the macro I saw in it does what it implies
<headius> yeah so it seems like they are doing what I described above, providing a way to walk the other module's hierarch in place
<headius> rather than copying it over at include time
<headius> oh I have a thought then
<enebo[m]> our problem is pre-3.0 so I think we don't emulate this behavior at all here
<headius> so we are still doing the "gather" and copy
<headius> so at include time there's only the module M itself right
<enebo[m]> yeah
<headius> so then back to searching for super... we walk into the M include wrapper and find it is prepended and then only get the prepend method table
<headius> so we only call the X method
<headius> so we need to go to M's method location I think, which should be the prepend shim where the methods were moved
<enebo[m]> we should be calling the M method because we did the prepend afterwards so A should not ever see it
<headius> this may be a problem with how we reference the module
<headius> we put a pointer to the module into the include wrapper, where I think MRI puts a pointer to the method table
<headius> so when the method table moves, they still have the right reference but we are pointing at the module itself, now gutted because of the prepend
<headius> we are trying to find the right method table at the moment of the call but they find it at the moment of the include and save that
<headius> I might be wrong about how they do includes but I remember somewhere that they only save a reference to the method table
<headius> again a case where our old logic was wrong because we copy AND delegate where 2.7 only copies and 3.0 only delegates?
<enebo[m]> well I think method table and constants table
<headius> right
<enebo[m]> rb_include_class_new
<headius> 3.0?
<enebo[m]> 2.6
<headius> ok
<enebo[m]> it seems to make a new class and gives it ivars, constants and ids (methods) from the original module
<enebo[m]> So those are probably shared pointers?
<enebo[m]> RCLASS_IV_TBL(klass) = RCLASS_IV_TBL(module);
<headius> yup
<headius> and they do prepend by moving the method table up to the prepend shim
<headius> so CRuby still has the reference to M's method table even though it is moved, and we try to find it at call time
<enebo[m]> yeah the shim is made by that method
<headius> and find the wrong one
<headius> we find the prepend table
<headius> so we call X instead of M
<headius> and then super proceeds up regular hierarch and we never see M
<headius> what is with my hierarchy spelling today
<enebo[m]> so our includedModule could just hold same Map as the original
<headius> "just"
<headius> I have been scared to make that change
<headius> but it is part of the root issue here
<enebo[m]> "JUST"
<enebo[m]> yeah I did not imply it is not scary :)
<headius> heheh
<headius> on the other hand it may be easier than we think because it kills logic we have to dig the method out of the included wraper
<headius> so we just go to included wrapper and use its table, done and done
<enebo[m]> yeah I was thinking of that in a positive and a scarily negative way
<enebo[m]> it is much simpler but it is removing a lot of little code snippets
<headius> jit fix turned out easier than I thought so I just did it
<headius> nice formatting on that code
<headius> er on that output
<headius> ah they must have added content type... three backticks with "text" renders right
<headius> but it should assume "text"
<headius> got a bit larger due to some refactoring but all cases I know of will have a proper backtrace element for arity errors
<headius> this may also have affected required and unknown kwargs
<headius> ah yeah, affected in the same way variable arity methods are affected (line number is because it was not set until after arity and kwarg checks)
<enebo[m]> looks reasonable
<headius> why is this failing on Windows
<headius> two commits that just fixed backtrace line numbers and somehow a tempfile test is failing