<puritylake[m]> enebo: is there an issue with overwriting methods on builtin types like `==`
<puritylake[m]> s/`==`/`==`?/
subbu has joined #jruby
subbu has quit [Quit: Leaving]
<puritylake[m]> Never mind I was being dumb
<puritylake[m]> Well I have fixed a few failing tests for arrays, a lot of what is left is the continuation based ones, I’m gonna attempt to fix those today
<headius> Good morning!
sagax has joined #jruby
subbu has joined #jruby
<enebo[m]> puritylake you can PR individual or sets of changes whenever. You do not need to make all test_array to pass
<puritylake[m]> Have the changes made in a branch, each set of passes are a separate commit
<enebo[m]> puritylake: ok well you can just PR that or wait...but it is not too hard to make new branches
<enebo[m]> puritylake: I think the main guiding rule is if you make n different changes and it will be difficult to see what caused some later issue from those changes it is nicer to be able to isolate them
<enebo[m]> with that said you have each fix as its own commit that does that
<enebo[m]> Biggest win on changes is being able to bisect. commits allow that.
<puritylake[m]> I can make them separate PRs that’s easy enough to rectify
<puritylake[m]> I’m not too well versed on how to structure these sorts of changes
<puritylake[m]> Probably should have asked in advance
<headius> no worries
<headius> Rails version:
<headius> Ruby version: jruby (2.6.8) 2022-05-03 aaef8ed95c OpenJDK 64-Bit Server VM on +jit [x86_64-darwin]
<headius> enebo: rails on 9.3 head looks fine for post scaffold crud
<headius> at least on sqlite
<headius> production looks good too
subbu has quit [Quit: Leaving]
<headius> sass fix in confirmed
drbobbeaty has quit [Ping timeout: 276 seconds]
drbobbeaty has joined #jruby
<enebo[m]> headius: cool beans
<headius> so I also tried generating gems for 70 and almost got something to scaffold, but it dies with "NoMethodError: undefined method `all' for ActiveModel:Module"
<headius> enebo: I think I will merge what I have in my rails 7 PR since I probably won't get back to this until my flights or after I arrive
<enebo[m]> ok
<enebo[m]> or both
<enebo[m]> headius: you mean local fixes to JRuby or arjdbc PR for rails 7?
<headius> arjdbc
<headius> I have a branch and PR for getting stuff to run better on rails 7
<enebo[m]> yeah I knew you had a PR for it but I wasn't sure if you have other fixes for JRuby itself
<enebo[m]> err I was wondering if you did anyways
<headius> enebo: welcome page failed with arity issues, a zsuper and some other initialize that received 4 args but wanted 3
<headius> I tweaked those two methods to work and welcome screen boots, so we're not too far off
<enebo[m]> yeah I should have that one done today
<enebo[m]> It is annoying because I need to build same if/else regular calls have for kwrest but the code is organized differently so it has a lot of changes
<enebo[m]> As far as I know though that is the only issue now with Rails and kwargs
<headius> yeah I won't poke at the arity issues but I will try to figure out that `all` error... probably missing some module include in an ARJDBC class
<enebo[m]> yeah remotely possible it is subclasses with include/prepend too
<enebo[m]> I think that happened post 2.7 so it shouldn't but who knows
genpaku has quit [Ping timeout: 240 seconds]
<headius> possible yes
genpaku has joined #jruby
<enebo[m]> headius: that fix did not fix Rails in arjdbc
<enebo[m]> headius: It was only for people using ARJDBC in a library capacity
<headius> eh?
<headius> oh I see
<enebo[m]> but it was broken
<enebo[m]> Rails 6 ran fine without the fix
<headius> ok
<enebo[m]> I believe the initializers fixes the adapter name or something like that so we still load properly
<enebo[m]> There is a weird muddle of features in arjdbc like the fact that mysql and pg will try and simulate cext APIs
<headius> I can tweet something to clarify, or not
<enebo[m]> nah. It doesn't matter
<headius> I don't think it matters... Rails 6.1 still working, Rails 7 on the way
<headius> ok
<headius> hmm the ActiveModel thing seems to be some scoping or constant lookup issue
<headius> it should fail to find ActiveRecord::Generators::ActiveModel and fall back on Rails::Generators::ActiveModel but the constantize logic finds the root ActiveModel module instead
<headius> `"#{options[:orm].to_s.camelize}::Generators::ActiveModel".constantize`
<enebo[m]> There are some constant spec failures where adding constants put them into the same place versus independent places involving singletons
<headius> that orm option comes through as active_record
<headius> hmmm could be related to that
<enebo[m]> I am not sure if that constant behavior is 2.7 or later
<headius> good news is with that tweaked in the generator logic, the scaffold generates and crud operations work
<headius> ship it
<enebo[m]> ok that is cool. I also have one significant unit test failure in active_model involving forwarding + zsuper
<enebo[m]> bwhahaha
<enebo[m]> It helps to actually add the instr vs just construct it
<headius> yeah this is great news
<headius> so once we fix these language issues things are looking surprisingly good
<headius> I have enough now to at least show it functioning, plus 9.3+6.1 to get some basic numbers
<puritylake[m]> enebo: To fix an issue with a test I edited RubyObect's method equalInternal
<puritylake[m]> This is the fix, it seems RubyInteger's own equality is ignored even when overwritten
<puritylake[m]> Preferring Fixnum's equality
<enebo[m]> hmm
<enebo[m]> I wonder if this is new behavior too
<puritylake[m]> invokedynamic mightn't be the right way but I don't know much of RubyInteger
<enebo[m]> puritylake: please land that as its own PR this is an important method
<puritylake[m]> I have been uploading all my fixes in separate PRs
<enebo[m]> oh ok great
<headius> yeah dunno if this is semantically correct, but what issue did it fix?
<puritylake[m]> headius: Here overwriting Integers == method doesn't actually get called by instead RubyFixnum's equality is called regardless
<headius> ahh eqq
<puritylake[m]> s/Integers/Integer's/, s/by/but/
<headius> er nevermind that
<headius> this is used for a lot of equality paths
<puritylake[m]> I won't push the fix then
<headius> puritylake: no no, it may be fine I am just trying to determine the right place for the fix
<headius> the fast numeric method called there is the problem, it is not using a dynamic call when the method has been replaced
<headius> trying to find the right place for this in CRuby though
<enebo[m]> whether a fix is landed or not it is hard to fathom someone overriding eqq on integer in practice
<headius> true
<puritylake[m]> Ya it does seem very unlikely someone would every need to do that BUT should we disregard it completely? There might be an actual use case perhaps in the future?
<headius> CRuby is ok with this I assume?
<puritylake[m]> Yup but I'm second guessing myself so one second while I double check
<headius> 8654
<enebo[m]> puritylake: yeah. the use case here will be some analytic tool which studies how often integers are compared of similar tooling
<headius> I will look into that
<headius> the fix doesn't appear to be related to dyncalling back into the object, it's just checking array bounds
<enebo[m]> puritylake: we generally fix all compat issues so I am not saying we shouldn't fix this but from a priority perspective if this is complicated we will tag it out and leave it be
<puritylake[m]> enebo: ya it isn't causing any major problems currently
<headius> oh I see
<puritylake[m]> Just one failed test
<headius> Array#count uses equalInternal directly which optimizes this dyncall out incorrectly
<puritylake[m]> Yup, the instanceof always hits RubyFixnum when a RubInteger is passed
<headius> yeah so their version of this is rb_equal_opt and it does check for overrides
<headius> ours doesn't
<headius> etc, and then if none of these core types have the original core method, it returns UNDEF here and falls back on a dynamic call
<headius> puritylake: the right fix would be to add checks into that fast numeric operation method that confirms whether the method has been overridden in Fixnum and Float
<puritylake[m]> Ok, seems doable, I'll work on that in a bit and push a PR later on tonight
<headius> there's an example of checking type AND isBuiltin using a call site
<headius> `context.sites.Fixnum.to_f.isBuiltin(arg)` is the important bit
<puritylake[m]> Duly noted, thanks
<headius> 👍
<enebo[m]> puritylake: I looked at the first two PRs and I notice a slight difference. What you did in the PR is probably how it should work but I think MRI only frozen errors when they decide to modify the array not when it logically should have
<headius> I believe that is correct
<enebo[m]> Both are basically the same fix
<enebo[m]> headius: It seems logical at an impl-level and more efficient but weird as a Ruby user
<enebo[m]> I could argue it both ways though. Until the array is really changed you should not error but on the flip side you should not have to know how an array is impld to know when it will raise
subbu has joined #jruby
<headius> enebo: I merged my arjdbc PR
<headius> I mostly did sqlite3 adapter copying but there are a few small fixes for other DBs to get them running
<headius> once the IR thing is fixed they may complete test runs
<enebo[m]> headius: heh my specific known cases are all working now but mspec is failing somewhere so 2 steps forward one step back
<headius> hah ok
<headius> I'm super excited to get my optimizing hands on all of these new arg forms
subbu has quit [Quit: Leaving]
<enebo[m]> headius: did you figure out the missing method ```"#{options[:orm].to_s.camelize}::Generators::ActiveModel".constantize```
<headius> It doesn't resolve the class right I think
<headius> The only ORM option I saw passed in was active_record so I modified the code to just access that namespace directly and it fails over to the rescue
<headius> The constantize operation seems to resolve the wrong module
<headius> Oh you know what, I bet it is a change in whether it goes all the way to the top level or not
<headius> So it looks for active model constant in the generators namespace and when it doesn't find it it keeps searching up and finds the root version of the module that doesn't have the expected method
<headius> So it could be a change in how they programmatically resolve constants and maybe Ruby 2.7 behavior
<enebo[m]> puritylake: I think that will always perform a frozen check now and not only if something had returned true enough to change something
<enebo[m]> ensure would also do the check if it always returned false in the block
<enebo[m]> It would still pass but I think it just means in most calls to this method it would perform an additional check
<enebo[m]> I think you want one checkFrozen right above the safeArraySet and a second one in selectBangEnsure but only if it will actually modify the contents of the array
<enebo[m]> That second change also fixes that part of select! too
<enebo[m]> I should have said "finally" and not "ensure" in that second sentence
<enebo[m]> whooo boi. I think I have a chance at passing spec:ruby:fast with this round of zsuper changes
<enebo[m]> this is some ugly stuff
<enebo[m]> replicated logic BUT it is enough different to replicate it. for zsupers from only methods we will specialize the super instr we use but if it is within a closure then we use a different instr...all the stuff around these instr selections are very similar but in closures you have to adjust all variable references back to source method and in method you have to wrap calls with the ability to catch breaks...and other diffs
<headius> Woot new laptop
<enebo[m]> nice
<enebo[m]> ok forward + zsuper is working
<enebo[m]> I reverse monitor.rb workaround and rails will generate an app now
<enebo[m]> active_model: 966 runs, 2850 assertions, 5 failures, 1 errors, 0 skips
<enebo[m]> The 5 F is expecting an order in a hash
<enebo[m]> The 1 is another arity mismatch bug
<enebo[m]> we stlll have a lot of method redefinition warnings
<enebo[m]> LOL NoMethodError: undefined method `now' for Time:Class
<enebo[m]> Whatever is causing that is messing activesupport results
<enebo[m]> welp Time is not getting ::Time even in places you would expect like stdlib logger
<enebo[m]> If I make it ::Time it works but it is not even in Rails...why would it not see ::Time.
<enebo[m]> anyways a lot of Rails 7 is working but there are some mysteries. Rails seems to really like specific order of things made from hashes so I am wondering how that all changed. We didn't use to fail tests like that.
<headius> back at my desk now
<headius> yeah that is weird
<enebo[m]> I restarted your arjdbc merge on GHA since I landed fixes for zsuper
<headius> it won't pick them up until tonight
<enebo[m]> I believe attrasgn will also have empty kwrest to be fixed now
<headius> jruby-head uses snapshots, doesn't build from master
<headius> unfortunately
<enebo[m]> oh I see
<enebo[m]> ok well not a big deal and I won't wait for it to finish now :)
<enebo[m]> I am going to continue tomorrow with more obvious things I can fix with basic language semantics and once I run out of those I will just hit things I see wrong in Rails/arjdbc
<headius> yeah we can see how it looks tomorrow but sounds like good news
<headius> yeah cool
<enebo[m]> active_model does have one arity problem and it is a long chain from a m_m with ruby2_keywords on it just passing *splat further up that chain
<headius> so we can probably say it is working at this point, and we have 9.3 still working to fall back on, so anything else is gravy
<headius> oh that sounds fun
<enebo[m]> I would think that should just work but whatever. This is getting better
<enebo[m]> The constant weirdness concerns me
<enebo[m]> not just Time but it seems we have other things at play here which looks weird
<headius> yeah maybe a worry but I think it is just some changes in how certain scopes look up
<enebo[m]> methods missing on some constants which seem a lot like Time not having now
<headius> like some scopes will scope to the block now and some scopes won't climb all the way to Object
<enebo[m]> I know singleton classes get their own constants
<enebo[m]> which probably is having it get its own cref or something like that
<headius> yeah
<enebo[m]> possibly just a clone of staticscope
<headius> push cref scope and that should work
<headius> cloned
<enebo[m]> yeah new or cloned I guess if we make lvars as a test that should tell us
<enebo[m]> It probably is same scope with its own cref
<enebo[m]> which would just be a clone
<enebo[m]> anyways...if I don't see you online tomorrow then I will talk to you later when you are settled in
<headius> yeah probably will be online some of the way
<enebo[m]> ok
<enebo[m]> dinner
subbu has joined #jruby
subbu has quit [Ping timeout: 260 seconds]
subbu has joined #jruby