<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