Successus has quit []
duane21[m] has joined #jruby
<headius> Good morning!
<headius> enebo: I'm going to land that null block PR today and look at other failures on master this week
<enebo[m]> headius: yeah it looks good
<headius> Ok
<headius> oh nice you merged 9.3 last week
<headius> I wanted to finish (or revisit) my fiber PR but will have to sync it with the vthread stuff
<headius> I'm going to look at this YAML CVE
Successus has joined #jruby
<headius> seems fine to update snakeyaml... faliures in PR are due to some regression on CRuby master, unrelated to snakeyaml
<headius> I recommended the patch be merged and released ASAP
<headius> byteit101: I see no down side to adding `foo=` for a non-void Java `setFoo`
<headius> commented on your proposal
<enebo[m]> don't we already do it for non-void?
<headius> we do not
<byteit101[m]> oh good. I'm scanning a larger m2 to see what would be affected
<headius> because JavaBeans did not work that way
<headius> but I don'
<headius> I don't see a problem with adding it... if you only want the assignment and don't care about the return value, you should be able to do `x.foo=`
<enebo[m]> wow. I totally remember this differently but it was more than a decade ago and I suppose I am not using setters that much now
<headius> yeah JB spec required that property setters return void or they would not be picked up
<enebo[m]> ah so I remembered it exactly opposite of the rule
<headius> not detected automatically anyway
<enebo[m]> So anyone doing var = x.foo= 1 should not expect 1
<headius> no
<headius> it will still be 1
<enebo[m]> Then even that will not break expectations
<headius> but if you do x.foo=(1) it will return the result of setFoo(1)
<enebo[m]> yeah I knew that I geuss :)
<headius> yeah
<headius> so if you want both foo= and the setFoo return value you can still get it with `.foo=()`
<headius> or just call `set_foo`
<byteit101[m]> Uh, what?
<enebo[m]> My only thought when I saw this is breaking some existing use but honestly there should be none
<byteit101[m]> isnt foo= and .foo=() the same call?
<byteit101[m]> if you want the return value, use set_foo
<byteit101[m]> if you want the ruby style, use foo=
<headius> byteit101: the syntactic sugar form is modified by the parser/compiler to always return the RHS
<byteit101[m]> see my last big comment on th eissue, specifically the footnote
<headius> the non-sugar form acts like a normal call
<byteit101[m]> Nifty! I didn't know that
<enebo[m]> I thought about adorning java with foo=() explicitly but that would still override the convenience member
<enebo[m]> only other thing is someone explicitly hoping foo= will fail with send....that is not an issue 😀
<byteit101[m]> enebo: yes, nothing should break as this is adding a new call where none existed previously. though if someone added foo= manually via re-opening the class, the new method would still be overwritten
<headius> it has come up in the past
<enebo[m]> byteit101: yeah that and send are all I could think of
<headius> we never had a good argument against it other than JavaBeans doesn't do it
<enebo[m]> Argument originally was following a convention everyone understood (and not something we would have to document)
<enebo[m]> but no one even knows what a java bean is any more
<headius> yeah that too
<byteit101[m]> yea, and now there are several good arguments in favor now that fluentish is a thing
<enebo[m]> So I am working on prepend/include updating includes/prepends on older calls
<enebo[m]> The simple explanation for how it works is modules will track a field subclasses (MRI field name) and then update those during the newest includes/prepends copy_init
<enebo[m]> but it is not so simple. Lots of tracking of whether what it saves is actual module or ICLASS
<enebo[m]> if it is an ICLASS it does more walking since it is not really the module to be included
<headius> enebo: this is moving toward having the modules be referenced from included hierarchies rather than copied, yeah?
<headius> so that future includes into those modules are seen
<enebo[m]> the feature is future includes are seen by previous ones and the mechanism is saving all those classes which included it originally in a list
<headius> right
<enebo[m]> but we do not copy for include
<headius> we should have that logic already in modules for invalidating method caches
<enebo[m]> I think though we do for refinements
<headius> i.e. changing a method in a module will invalidate any hierarchies where the module was included
<enebo[m]> yeah there is a method invalidation section added for this but we will need to invalidate as well
<headius> so just adding that same behavior for structural changes to the module's hierarchy... if it gets an include, its includees must be invalidated
<enebo[m]> I expect there to be things to figure out but I am just trying to understand the ICLASS stuff atm
<enebo[m]> headius: is invalidation just calling each subclass being changed with invalidateCacheDescendants?
<enebo[m]> I wonder how close includingHierarchies are to what we want
<headius> that should be it
<headius> every module keeps a weak list of classes where they are included
<enebo[m]> I do not see how that is added with prepend
<headius> IncludedModuleWrapper constructor does origin.addIncludingHierarchy
<headius> and is used by both prepend and include
<headius> prepend may be a little weird because the origin moves around a bit
<headius> but it's in there
<enebo[m]> ah I see it
<enebo[m]> prepend will eventually proceedWithInclude
<headius> right
<headius> a tangled web but it all bottoms out in IncludedModuleWrapper eventually
<enebo[m]> yeah and I prefer ours to MRIs which combines the two into a single method
<enebo[m]> Even if the logic is remarkably similar it still is not if this particular value is null it just happens to be prepend logic
<enebo[m]> I half wonder if AOP was an early attempt and solving this two very similar methods
<enebo[m]> Closures definitely could make some common code but in both cases the cure may be worse than the disease
<headius> Yeah I don't know how much you would gain by trying to combine them
<headius> Mostly avoiding fixing the same bugs twice I guess
<enebo[m]> forgetting one location
<headius> Yeah
<enebo[m]> at the cost of knowing which branches in the method are specific to one or the other
<enebo[m]> interesting tradeoff
<enebo[m]> although they could use names "I" understand
<headius> The cruby code is kind of inscrutable because they combine so much logic
<enebo[m]> const VALUE klass, VALUE c, VALUE module,
<enebo[m]> heh what is c and what is module?
<enebo[m]> if a prepend they are always the same
<enebo[m]> but yeah inscrutable
<headius> Yeah I would love to know what drives them to use single letter variable names at all
<headius> Lunch for me, and then I'll refresh my memory on what still fails on Master
<enebo[m]> I guess 'p' is ok if it is just a pointer but it is still weak sauce
<enebo[m]> json parser walking down a stream of chars I will let 'p' slide
<headius> enebo: did you open an issue for that timeout thing?
<enebo[m]> headius: I did not
<headius> enebo: ok
<headius> in looking at current failures I was reminded that this is causing most -Ptest runs to fail so I'm trying to finalize that change
<enebo[m]> yeah it is the only failing issue
<enebo[m]> sometimes java11 finishes in time
<headius> modified based on review, should be ok to merge and release now
<headius> yeah it is just lucky if the thread completely shuts down before we check for dangling threads
<headius> new impl will close the timeout request queue and join on the timeout thread so we know it is gracefully shut down
<enebo[m]> yeah saw the changes to the PR but I saw the Pr last week
<byteit101[m]> kares: Yes I have no intention of adding setter with more than one parameter
Successus has quit []