<headius>
Matt Welke: Nice! So the idea here is just plain Java sources packaged up and shipped in a gem, yeah?
<headius>
I was going to say that PR above is a good example of building a JRuby ext, but this is not an ext, it's just a Java library to be consumed from JRuby
<MattWelke[m]>
Not sure what a JRuby extension is right now.
<headius>
extension = using JRuby Java APIs to integrate directly into Ruby, like String or Array or Hash
<headius>
eliminates the overhead of crossing Java/Ruby boundary but requires a lot more knowledge of JRuby internals
<MattWelke[m]>
But yeah my goal last night was to get a basic idea of how to make a Ruby gem (up until then I'd only published NPM and Go libraries) and once I had that working, I was curious what it would be like to include some Java in it for use in JRuby. I ended up being successful after a lot of trial and error and following the rjack GitHub repo as an example.
<MattWelke[m]>
For example, one weird thing I noticed was that I had to use a package in Java. I've been able to execute class files where no package was used in the Java world, but in JRuby, it said "unitialized constant" when I'd call like `Hola.hi`. It only worked when I used a package and called it like `com.mattwelke.javagemtest.Hola.hi`.
<headius>
honestly if the Java part is standalone you might be best off publishing a Maven artifact and then linking that in with jar-dependencies (RubyGems plugin we provide that allows fetching and installing Maven artifacts alongside gems)
<headius>
ahh you can access the default package from JRuby but it is not recommended in the Java world these days
<MattWelke[m]>
true. I just like to start minimal and then build my way up.
<MattWelke[m]>
The extension method you describe sounds like something worth building up to now. Less overhead sounds good.
<headius>
sure, this is a very simple way to get some Java code into a gem
<MattWelke[m]>
And afaik this is how things like ActiveRecord are implemented in JRuby?
<headius>
for AR we have an extension, since there's a ton of Ruby types being tossed around and converted and such
<headius>
logically it is just a wrapper around JDBC though
<MattWelke[m]>
> honestly if the Java part is standalone
<MattWelke[m]>
actually, nevermind. Lost my train of thought. I don't see any advantage to them not using extensions based on my understanding so far.
<MattWelke[m]>
I just liked the pattern I saw in that repo. They just ship the JAR with the gem and the main Ruby file for the gem `require`s it. That way you don't need to use any tool other than bundler to get the JAR.
<headius>
so this is definitely a way to repackage Java libs for use in JRuby
<MattWelke[m]>
I know people there said to check out jar-dependencies. But I happened to stumble upon the rjack repo first.
<headius>
they appear to be mostly maven builds that will fetch the library in question, and then some bits to turn that into a gem
<MattWelke[m]>
Exactly. And because I like to start with minimal tests and then build up, I wasn't using Maven yet. Just javac and jar.
<headius>
historically we used to recommend people re-release Java jars as gems, but got away from that because there ends up being dups and none of them are canonical (like fetching from Maven itself)
<MattWelke[m]>
> repackage Java libs for use in JRuby
<MattWelke[m]>
This might be the right approach for a technique I wanted to try then. Remember me saying I thought it'd be neat to make data pipelines in Ruby with Apache Beam? I wouldn't need high performance because the Java code in Apache Beam is just called to set up a DAG. It's the Beam internals that do the work.
<MattWelke[m]>
So maybe wrapping the Beam JARs, either plainly like in my example where the JRuby user is expected to call Java, or richly where I make some Ruby methods people can call, would do the trick.
<headius>
for a library like that I would just use their jars directly, either republished or via jar-dependencies, and then a thin wrapper in Ruby to make it nice
<headius>
doesn't sound like there's need for a tightly integrated extension
<headius>
depends on what data is crossing that boundary mostly
<MattWelke[m]>
Do you have an example I can look at for making an extension instead of the way I repackaged the JAR?
<headius>
(i.e. would it want to be able to use Ruby String data directly or is copying into a Java String first ok)
<headius>
hmm
<headius>
well the psych gem (Ruby yaml lib, wraps libyaml in C and SnakeYaml in Java) does both: jar-dependencies to fetch the SnakeYAML library and an extension to provide the Psych API
<headius>
github.com/ruby/psych
<headius>
it is a pretty clean example of wrapping a Java lib with a (mostly minimal) extension
<MattWelke[m]>
Does jar-dependencies come into play when you're making an extension?
<MattWelke[m]>
Trying to wrap my mind around the various ways to call Java from Ruby here... Want to play with each one.
<headius>
it does if your extension depends on existing Java libraries that are published with Maven
<headius>
if your extension is just a standalone piece of Java code there's no real need for jar-deps
<MattWelke[m]>
And an example of "standalone piece of Java code" might be a Ruby gem that for performance reasons, had a C extension in the MRI world? So in the Java world, the C extension would be ported to Java and made into a Java extension?
<MattWelke[m]>
s/Java/JRuby/
<headius>
right
<headius>
the port of "oj" that enebo has been working on is a good example.. standalone hand-written json library in C ported to Java
<MattWelke[m]>
And we can expect there to be some situations where a Java extension would be more performant than a "pure Ruby" gem where we relied on JRuby to run the Ruby code? I imagine it would be the same use cases where in the MRI world, they opt to make a C extension.
<headius>
very similar yes
<headius>
JRuby will run Ruby code faster in general, but sometimes you just have to push bytes
<MattWelke[m]>
Alright I've got lots more to play with now. ty for the advice :)
<MattWelke[m]>
Sorry for any dumb questions. I haven't done Ruby or Java since 2016, in school.
<headius>
FWIW I know we don't have very clear messaging here and we want to fix that... JRuby has been around a long time so there's lots of different conflicting approaches
<MattWelke[m]>
Don't ask me why JRuby appeals to me so much for learning both deeply. I just like to challenge myself I guess. xD
<headius>
welcome to the party pal ð
<MattWelke[m]>
One of my main areas of focus is docs btw. My company thinks of me as the "docs guy" and I do a lot of training for new hires.
<MattWelke[m]>
I'd be happy to contribute back guides once I sort all this out myself, assuming I stay interested in JRuby.
<headius>
we have lots of docs, they are just old and conflicting and hard to navigate ð
<headius>
when given a choice between fixing a user segv or updating docs I kinda have to fix the segv
<MattWelke[m]>
yeah makes sense. I don't blame you
<MattWelke[m]>
work time, ttyl
<headius>
ttfn
<headius>
I've caught up a few issues so I'm going to reward myself with some 3.0 work
<headius>
enebo: have you made any improvements to kwargs on master?
<enebo[m]>
nope
<enebo[m]>
It is a big ticket item I have not had the energy to attempt
<headius>
ok, maybe you have some guidance on what is there and what is missing
<headius>
I can try to do what I can
<enebo[m]>
yeah I think mostly just reviewing the binding changes
<enebo[m]>
shoot HAHA I need to find the YUGE issue which talks about them
<headius>
hmm yeah this really needs changes in the nodes and parser
<headius>
well maybe... ArgsNode right now for example just has an array of args and a keyword offset, but the new way is that keyword args are explicitly separate
<enebo[m]>
Well I think we have thatalready
<headius>
public ListNode getKeywords() {
<headius>
return new ListNode(getLine()).addAll(args, keywordsIndex, getKeywordCount());
<headius>
}
<enebo[m]>
Or at least literal kwarg syntax is there
<enebo[m]>
but I think the main work will be in changing the plumbing of calls
<enebo[m]>
since when we know we are passing kwargs in a call we would like them to be out of band
<headius>
I should dig up some failing specs and start walking through it
<enebo[m]>
or a known position
<enebo[m]>
but detecting literal kwargs I think is there. It is more about I have kwargs and I want to make a call
<enebo[m]>
So on native side it would be cool if we had something which would just recieve it into the method
<enebo[m]>
On interp/jit side that whole frobnicate and to_hash and all that probably needs a change
<enebo[m]>
if we have a call to a Ruby implemented method and it passes a kwarg we know it is a kwarg and so we won't be to_hash'ing in that case. We will still need to make sure all required kwargs are there
<enebo[m]>
The other part is that there is ruby2_keywords which can be marked on a hash
<enebo[m]>
Somewhere internally I guess we off-ramp that hash into the out of band param piece of making a call
<headius>
hmm yeah the native piece
<headius>
that will be nice to have but CRuby has nothing like that
<headius>
on the native side they still largely just look for kwargs in last arg slot or something
<enebo[m]>
I do foresee call/finvoke to all accept an additional param so essentially bifurcating all those methods :(
<enebo[m]>
Actually I was going to say it can just be one param but ideally we don't want that
<enebo[m]>
we probably want some metadata saying what the values of the kwarg are and positionally pushing those as args
<headius>
most of those paths will have to box and unbox except for the most trivial cases
<enebo[m]>
I think this is something we can possibly change/evolve over time too
<headius>
like we could have finvoke(..., key, value) and finvoke(..., key1, value1, key2, value2) but obviously can't take that too far
<enebo[m]>
yeah this also means we need an int for how many real args and then something to process those kwargs
<headius>
arg forwarding is also failing a lot of stuff
<enebo[m]>
arg forwarding is broken completely
<enebo[m]>
err well for anything involving ** or &
<enebo[m]>
that is because we do not have kwargs working
<headius>
ok
<enebo[m]>
The parser was making invisible variables for those two but it broke more because kwargs is not working
<enebo[m]>
in the parser forwarding is just making some unseen variables to hold the three values you will forward
<enebo[m]>
IR building will just take those and then the forwarded call will get them as params
<enebo[m]>
so it is simple to turn fully on but we have to know how we plan on passing kwargs
<headius>
well perhaps we should also talk about how to represent call args in a squashed format without losing structure
<headius>
IRubyObject[] is not sufficient to keep kwargs and positional separate
<headius>
and we have to have a squashed way unless we are adding a separate kwargs arg to every call path
<headius>
that might be doable in the long term but clearly not this month
<enebo[m]>
yeah I am not sure
<enebo[m]>
Doubling all our call signatures is possible I think but I am not sure it is where we want to be in the end
<headius>
if we had an analog to Signature that was Arguments with separate slots for positional and kwargs that would be a lowest common denominator for passing and forwarding
<headius>
heh ArgsNode does receive a list of keyword args in one constructor but shoves them into the same array, is this just a space-saving measure?
<headius>
perhaps that is a place to start
<headius>
I see IR also has logic for looking at the last normal arg of a call and treating it as "maybe kwargs" if it is a hash, so that is clearly changing in 3
<headius>
yeah so starting at this low level it already is munging positional and kwargs together
<headius>
I think we need to start here and in parser and keep keywords as a separate item rather than a HashNode
<enebo[m]>
I have an idea
<enebo[m]>
and yeah how we handled kwargs for pre-3 is just different
<enebo[m]>
I think we can make a RubyKeywords implements IRubyObject and just pass it as last arg
<headius>
I guess I am saying I'm not sure whether I can start to fix stuff in IR and beyond unless the parser is keeping these separate to begin with
<enebo[m]>
We can also make an Operand Keywords which can cache this
<enebo[m]>
Since it will have no side-effects
<headius>
yeah that is along the lines I'm thinking
<headius>
elevate kwargs to its own construct everywhere and then we are not playing this guesing game
<enebo[m]>
ok I had thought about this before but I had not thought about the idea we can keep a cached box because we know we are only going to be using it for a single call which will extract what it uses
<enebo[m]>
if we pass on kwargs it will have to put those into another keyword which will make another cachable instance
<enebo[m]>
so there will be some assignment cost but no allocation cost
<enebo[m]>
in literal kwargs builds it will have no cost and if they are literals not even assignment
<enebo[m]>
(on call set up side)
<enebo[m]>
I may be a little too optimistic here
<enebo[m]>
The values themselves we will be pushed in so they will not need to change the box but that site may be happening in parallel so for things like foo(a: some_meth) we have to remake the box over and over since some_meth can return anything
<enebo[m]>
but foo(a: 1) can just be alloc'd once forever at that site
<enebo[m]>
For IR I think we just need Keywords operand and the AST keyword keys and values emit into assigning to it:
<enebo[m]>
In the second file in that gist you will see what will actually happen since the fixnum will prop
<enebo[m]>
If all args prop we can optimize to only allocate a single instance
<enebo[m]>
lunch
<headius>
yeah so having a per-context carrier object would be helpful for many things
<headius>
it delocalizes the args from the call and the receiver (damaging escape analysis and what have you) but this is the default path... specific calls+receiver combinations will just pass things on the stack directly and avoid the box altogether
<headius>
but the cached box avoids allocation; caller fills the struct, receiver empties it
<headius>
then for arg forwarding we always have a structured way to pass the same argument layout forward
subbu has joined #jruby
<enebo[m]>
back
<headius>
So am I right in thinking that in 3.0 all calls will explicitly separate positional and keyword arguments with literal hashes just being normal positional arguments
<enebo[m]>
compile.c I think does do that
<enebo[m]>
for some things
<headius>
The only magic happens on the receiving side, which for normal 3.0 methods will only accept the literal keyword arguments as keyword args and for receivers marked as ruby2_keywords use the maybe logic to unbox any passed in hash
<headius>
I guess what I'm getting at is that we really need to start at the beginning of the call pipeline and make sure keyword arguments are being separated all the way through into IR because there is no other representation we need to maintain
<headius>
All calls either have keyword arguments and we know their names or don't have keyword arguments and pass only positional
<enebo[m]>
flag |= VM_CALL_KWARG;
<headius>
With the special dispensation for rest keyword args but they still are considered separately from positional in all cases except once we get to the receiving side
<enebo[m]>
I believe adding a keywords operand in IR allows us to represent anything but its real benefit is we represent it live as something masquerading as IRubyObject but not used as IRubyObject
<enebo[m]>
Signature verification will need to args[args.length - 1] instanceof RubyKeywords or something like that
<headius>
Keywords operand I agree with, or at least something specifically keyword related in IR The values can be arbitrary expressions so I'm not sure an operand will be enough
<headius>
The metadata for the structure of the keywords is static but the values are calculated at runtime
<enebo[m]>
well we have Array as an operand but the array can have anything expanded into it.
<enebo[m]>
I think they are a direct analogue
<headius>
Yeah in that case it's just an array of references to temporary variables
<enebo[m]>
but there is a hybrid here that you can end up generating an immutable keywords if all values are not variables but literals
<headius>
So the argument values get calculated and shoved into temps and then yanked out for the keywords operand during the call
<enebo[m]>
in the interpreter they would for sure
<headius>
Oh for sure, that's how we're going to eliminate all allocation for things like the exception keyword that's all over IO now
<enebo[m]>
for the JIT you can possibly unwrap the box into something else?
<headius>
exception: false doesn't need to allocate anything to hit the right logic
<headius>
For jet, what I want is the metadata so I can embed a descriptor into the dynamic call and pass the values as plain old Java arguments on the stack
<enebo[m]>
yeah that case should work great with keywords operand ebcause we make one live instance of RubyKeywords and we know its contents never change
<headius>
For jit
<headius>
What do you mean by that? At minimum it has to be per thread
<enebo[m]>
keywords([:exception] [true]) -> we know it never changes we can share it across anything forever
<enebo[m]>
simple literal values in IR will propagate
<headius>
For that case yeah, and if we are calling an old style method we just create the hash at that point and populate it
<enebo[m]>
but are their old stle methods?
<enebo[m]>
there are old style arguments but is there methods?
<headius>
If they are flagged with ruby2_keywords
<headius>
I'm not sure what the difference is
<enebo[m]>
oh it has been a while...I did not (and do not) remember that working for method definitions
<headius>
Yeah it started out as a way to mark a method as using the old style where any arbitrary hash passed in can be interpreted as keywords
<headius>
It's used in the same way as public and private
<enebo[m]>
ok I did not remember this detail but those methods obviously will need to process the static incoming keywords and make them splatty
<headius>
The marker on a hash is used to indicate whether the hash was created to support old style keywords I think
<headius>
That part isn't clear to me just yet
<headius>
In any case I don't think there's any ambiguity on the call side and keywords there are explicitly kept separate
<headius>
And only support the literal syntax or splatting out of a hash
<enebo[m]>
I think from IR side of things an operand Keywords which is similar in idea to Array but where we can detect immutable Keywords being an optimization we can pass it through ordinary call path and the special object type can be easily noticed
<enebo[m]>
My only fear with this design is native core methods which do not expect to see those. In that case I think we just make it emulate a hash in behavior
<enebo[m]>
That might have a positive side-effect too...All our old core impl logic will just continue to work (if RubyKeywords extends RubyHash)
<headius>
Core methods would just be implicitly using the old style unless we specify the new style and provide a parameter or parameters to put the keywords in
<headius>
Or we go around and mark every method that receives keywords with an old style flag right now and start moving the default behavior toward having individual Java arguments for keywords
<enebo[m]>
but if we had a RubyKeywords which is a RubyHash (and add fields or what not) we could possibly never have to even change the core methods
<enebo[m]>
I personally would like to change them but I am also thinking about this as a faster effort
<enebo[m]>
We can always add a better mechanism later via populators/annos but something which would work with the existing impl would be nice since it would eliminate a lot of short term work
<enebo[m]>
oh man I forgot I was fixing Dir#glob bugs last week
<enebo[m]>
talk about a scary place
<headius>
write a compiler
<enebo[m]>
ok
<headius>
so actually my first model is probably best
<enebo[m]>
I got it down to 3F but it is a single failure where {}{} need to not sort but the things matching inside them do sort
<headius>
unless you specify that this arg is this keyword then we just assume you are receiving ruby2_keywords style and will sort it out yourself
<headius>
that will let the existing methods work as they do today and when we have a way to split off keyword arguments on java arguments it will just switch
<headius>
hah well that is useful ð
<headius>
I guess I can start looking at this by creating the IR operand and trying to plumb it
<enebo[m]>
So what about idea of a masquerading IRubyObject with the keys/values in it?
<enebo[m]>
Or do you just want to start by using an actual RubyHash
<headius>
what does that save us over using RubyHash?
<headius>
any impl of IRubyObject has like 100 methods to implement
<enebo[m]>
I was going to say we could specialize lookup by having it in a field but we still have to determine the field
<headius>
I mean we certainly could make it a marker subtype of RubyHash too, I'm just not fond of trying to fake out IRubyObject and having it leak and go weird on us
<enebo[m]>
and I think nothing would prevent that from being a very specific case and not the generic one
<enebo[m]>
I guess the other piece is knowing for sure it is a keyword arg
<headius>
if you mean using this as an out-of-band way to pass kwargs I think that would just be best on ThreadContext
<headius>
the Arguments struct I was describing earlier
<enebo[m]>
signature verification for example
<headius>
what do you want IRubyObject for here exactly? Just so we can pass through generic call paths and not lose the keyword-ness of it?
<enebo[m]>
For mutable keyword values you sort of have to use TV
<enebo[m]>
for immutable you would not neccesarily but I am not sure if they actually helps
<enebo[m]>
yeah I think if we are in Proc#call we need to easily know the last argument is a keyword or not.
<headius>
if this is for passing fully-literal kwargs we could have another structure, that would be useful... the way we do this right now is by having RubyHash create a "packed" version of a hash that stuffs everything in one bucket
<headius>
which is basically then just an array of key/value pair objects
<headius>
more efficient would be if we had a direct addressed hash
<headius>
or as you say, fields
<headius>
OneKeywordHash that just has a key and a value field
<enebo[m]>
yeah
<headius>
if this is the key I want, that is the value I want, and no lookup
<headius>
and keys are always symbols so idempotent
<enebo[m]>
yeah
<enebo[m]>
this is the whole exception: false case
<headius>
right
<enebo[m]>
So one place I see a benefit or maybe even a compat issue is Signature.checkArity
<headius>
hey refresh my memory why we sometimes have an ArrayNode and sometimes an ArgsNode
<headius>
ArgsNode is the receiver side?
<enebo[m]>
we have to know that if the signature requires a kwarg it is really a kwarg and not a hash
<enebo[m]>
in 2.x we don't care
<headius>
it is confusing because CallNode has a field called "argsNode" which will usually be an ArrayNode, right?
<enebo[m]>
heh I have to remember as well
<headius>
that nomenclature might be nice to clean up some day because it always confuses me when I return to this level
<headius>
DefNode.getArgsNode returns ArgsNode
<headius>
CallNode.getArgsNode seems to only return an ArrayNode, or perhaps a single argument's node
<headius>
again this is where I am wondering if I'm getting from parser what I need... I really need to know that these are the positional args and these are the keyword args before I dive into the call path
<enebo[m]>
ah ok I was just rooting around in the source
<headius>
like that paste above... a call with two positional and one keyword arg just looks like a call with an array with two args and a hash
<headius>
that's not good enough
<enebo[m]>
CallNode is just a list of things to pass to a call
<headius>
though HashNode does have a flag to indicate kwargs
<headius>
right but it is treating kwargs as fuzzy
<headius>
it should be strict now
<enebo[m]>
but for kwargs and block are tacked onto it
<headius>
it should be getPositionalArgs with an array of them, and getKeywordArgs with a set of k/v pairs in some form
<headius>
I'm just brainstorming here but most of this work is on the call side, explicitly peeling off keyword args and deferring their handling until we have a receiver
<headius>
it is how we wanted kwargs to optimize but could not (or at least it was tricky) because of the fuzziness
<enebo[m]>
Interesting. This is just how it is done in MRI whereas on def side it does have a little more complexity since there are different types of things it accepts
<enebo[m]>
I know in one case the issue is that the grammar figures out the elements in the reverse order so they/we make a holder object
<headius>
hmm
<headius>
so does that mean they are just treating it as the last arg with some marker?
<enebo[m]>
Let me look at their parser but I think this will be the same since this is all _ method names
<headius>
I think we can infer where the kwargs are from the current structure, but it is a lot of manual inspection of this arg array
<headius>
it would be nice if I could just CallNode.getKeywordArgs because then I can plumb them straight through to a receiver, or create a hash lazily for old style
<headius>
MRI is not the best example perhaps because everything is just a ball of crap
<enebo[m]>
hahah ok...this is because in MRI hashes are made out of lists
<enebo[m]>
so assocs makes a list and then anything using assocs makes the hash at that point
<enebo[m]>
I just make the hash at assocs
<enebo[m]>
so a difference without distinction
<enebo[m]>
anyways call could be made to be {position, kwargs, block}
<headius>
ok I think I was wrong
<headius>
I don't think I can accurately infer from the current AST that kwargs are kwargs
<enebo[m]>
yeah I am confused how they infer it
<headius>
it has this "maybe" logic that checks for all symbol keys, etc, but that's not a guarantee
<headius>
on the receiver side it is separated in ArgsNode so I know where the keyword metadata is, but on call side it is just an opaque hash at the end of the args array
<headius>
example of something failing because we cannot make this distinction right now
<enebo[m]>
nice. I am glad we made it past this hurdle
<enebo[m]>
each little detail clears things up a bit more
<headius>
if we can get the parser side hooked up right I think the rest will start to fall out quickly
<headius>
it is a simplification overall but some work to get rid of all this maybe logic
<nirvdrum[m]>
It sounds like you have a plan, but in case you didn't know, Chris and another Shopify employee have been working to improve kwargs in TruffleRuby. They hit a few hurdles along the way and I think they've managed to clear them. I'm sure he'd be happy to share ideas if you run into similar issues.
<headius>
as part of 3.0 work or just in general?
<headius>
we are not considering optimization at this point, but that will fall naturally out of having explicit keywords on call and receive sides
<headius>
I already know how to plumb keyword args straight through with no allocation but I need it to be explicit
<nirvdrum[m]>
In general. Although, the 3.0 semantics are less wonky than 2.7, so they've decided to adopt 3.0 and drop their work on supporting 2.7 semantics.
<enebo[m]>
good call
<headius>
yeah I gave up trying to make 2.7 kwargs optimize because of this wonkiness
<nirvdrum[m]>
The general idea is to make kwargs no more expensive than positional args.
<headius>
it should be easy now... just include metadata that describes the call stack and where positional args and key/value args are located, and unravel it on the receiver
<headius>
right, that should be easy with invokedynamic
<headius>
they will be exactly the same as positional args through to the receiver, and as long as the receiver also has explicit kwargs there's no alloc
<nirvdrum[m]>
I think they're mapping kwargs to synthetic positional args and storing a mapping in the method. But, don't quote me on that. There's been a bit of experimentation over the past couple of months and I'm not actively working on it.
<headius>
yeah, sounds like my impl
<headius>
if I run into any snags I'll ping Chris about it
<headius>
indy makes it pretty easy to mark up a call with metadata, so then it is just a matter of routing the positional args to either keywords on the receiver or lazily standing up a hash
<nirvdrum[m]>
Sounds good.
<nirvdrum[m]>
Some of it really sucks from what I gather. Good luck.
<headius>
in 2.7, it definitely does
<headius>
I believe it will be vastly simpler in 3
<nirvdrum[m]>
I think there was some work around storing data at the call site rather than the method, too, to deal with optional args. But, yeah, Chris would be the guy to ask. I've exhausted my knowledge on the topic.
<enebo[m]>
I believe we need a flag on RubyHash
<headius>
that's what I'm hoping enebo can provide for me from the parser
<headius>
really has to come from parser, so if y'all have parser changes to support this it would be great to see them
<headius>
in any case I can take it up with Chris as needed
<headius>
enebo: if we continue overloading RubyHash to also carry literal kwargs, then probably yes
<headius>
I do not know how that dovetails with the ruby2_keywords flag
<enebo[m]>
during signature checkArity we just look to see if the last argument is a Hash and if not try and make it one. That seems like that is not 3.0 behavior
<enebo[m]>
So we need to know if the last arg is actually kwargs or a hash
<headius>
yeah that will all change
<headius>
arity for non-ruby2_keywords methods will explicitly consider positional and keyword args separately
<enebo[m]>
but I thought you were just saying we didn't need to extend RubyHash
<headius>
well, I said I don't want some new synthetic IRubyObject dummy struct
<enebo[m]>
oh I revised that a little later to say it should extend RubyHash since then it would work with existing native core methods
<headius>
RubyHash with a flag or KeywordHash < RubyHash would be fine
<enebo[m]>
ok
<headius>
ok yeah I missed that
<enebo[m]>
yeah I only made the comment because I thought you were against the extended version but it is all clear now
<enebo[m]>
I pushed
<headius>
nirvdrum: and thanks btw, I know that kwarg optimization work has been ongoing in TR but good to know they have refocused on 3.0 behavior
<headius>
incidentally, though, shouldn't escape analysis and partial evaluation be eliminating these hashes for you?
<headius>
I thought that was the answer to stuff like this
<headius>
enebo: ok
<enebo[m]>
if (hasKwargs() && !TypeConverter.checkHashType(runtime, args[args.length - 1]).isNil()) {
<enebo[m]>
I think we do this a few times
<headius>
heh I do like isLiteral better than "isBrace" but MRI folks pick weird names sometimes
<enebo[m]>
It will be really nice to kill this off
<headius>
oh yeah we have half a dozen different flavors of that all over the place
<enebo[m]>
yeah brace was too strange to me
<enebo[m]>
if (args.length - 1 > required() + opt()) {
<enebo[m]>
we will still have this stuff happening
<enebo[m]>
unless we went full out of band like Block
<headius>
we need a better AST printer
<enebo[m]>
ah I will update HashNode to print that out
<headius>
good luck, I just realized all AST printing relies on the same generic Node.toString
<headius>
we don't even have an AST printing visitor that lets nodes print themselves
<enebo[m]>
yeah I thought it was written around providing extra info
<headius>
oh I see, there is toStringInternal that can be overridden
<enebo[m]>
and I think toStringExtra
<headius>
we only do that in one place
<headius>
toStringExtraInfo
<headius>
yeah
<headius>
fill that sucker up with info
<enebo[m]>
coming to your repo soon
<enebo[m]>
ok I added it and it is 100% marked as !literal
<headius>
sweet
<headius>
I am scrubbing out the maybe logic as I go
<headius>
hmm IR is still seeing `call({b:1})` as kwargs though, I didn't break enough stuff
<enebo[m]>
headius: are you seeing literal ever set?
<headius>
I don't think so
<enebo[m]>
I am confused. I can see that field getting set but then by the time I print out the ast it always says kwarg
<headius>
call({b:1}) still has literal = false
<enebo[m]>
I think something must be making a new hashnode from an old one?
<enebo[m]>
Looking into it
<enebo[m]>
what the hell
<enebo[m]>
I did not even put this remotely in the right place :)
<headius>
have another cup of coffee
<enebo[m]>
Seriously
<headius>
half these specs will pass if we can just get the hash marked as literal
<enebo[m]>
My brain cannot even process this
<headius>
because it will trigger arg errors
<headius>
actually maybe even more will pass because once we are beyond the call we know we have a kwargs hash versus a normal hash and only kwargs hashes will get sent through as such
<enebo[m]>
hahaha
<headius>
receiver side will need massaging but they will act like ruby2_keywords for now
<enebo[m]>
yeah I knew there was a lot hinging on telling the difference
<enebo[m]>
but we will also get signature mismatches which will be easy once we can look at a type or field
<headius>
right
<headius>
and we just wire it up as we go
<enebo[m]>
Last weds I did knock off about 30 F/E on ruby/spec so no doubt that doubled with MRI
<enebo[m]>
Those glob sorts are way more complicated than we have it impld too
<headius>
a large percentage of remaining fails are due to kwargs and forwarding being broken
<headius>
and the rest are pattern matching ðĪŠ
<enebo[m]>
not many though
<enebo[m]>
I made thousands of pattern matching tests work
<enebo[m]>
I should say I made them all work until we updated stuff
<headius>
ah nice, wasn't sure how far you got into them
<headius>
I just noticed a block of fails in the language specs and didn't really look too hard
<enebo[m]>
yeah anything there is from a single experimental feature of matching or just a bug
<enebo[m]>
I pushed the fix so kwarg/literal works
<headius>
yay
<enebo[m]>
and displays which would have been a nice thing to have done first
<headius>
just in the first couple layers after parser this already cleans up a bunch of logic
<enebo[m]>
ah yeah went through my last spec/ruby run and we fail 1 test in pattern matching
<headius>
ok so I guess next thing is that I need to identify a RubyHash that is kwargs
<enebo[m]>
Actually I expect this kwargs stuff will fix about <40F/E on ruby/spec
<headius>
so that will be the subclass marker or flag
<enebo[m]>
but it is important
<enebo[m]>
mri may have a lot more tests for it too
<headius>
maybe not many specs directly fixed but there are a lot of case expecting 3.0 kwarg behavior within a test for other behavior
<headius>
yeah more in MRI suite
<headius>
some of them are failing in the harness or in setup/teardown
<enebo[m]>
but a number of these are the worst ones because they will break generic code
<enebo[m]>
Just in case anyone was curious about pattern matching
<headius>
I was curious
<enebo[m]>
I also experimented with some new syntax in builder
<enebo[m]>
label as lambda helper looks pretty nice
<enebo[m]>
but I did several other closure-based helpers
<headius>
I will this work to a branch and we can debate some choices
<headius>
I have added a quick KeywordHash subtype of RubyHash and now the places where we try to dig it out of call args just need to check instanceof
<enebo[m]>
nice
<enebo[m]>
it is a simple way around things for now
<enebo[m]>
old code in core methods will still mostly just work
<headius>
yay, I'm as deep as frobnicate
<headius>
couldn't figure out why my keyword hash was a regular hash on the other end
<enebo[m]>
oh I guess there is another 200+ lines a bit lower for pattern matching
<headius>
subtype of RubyHash is tricky because we dup them all over
<headius>
so the KeywordHash has to know how to dup as another KeywordHash
<enebo[m]>
hmm
siasmj has quit [*.net *.split]
greyrat_ has quit [*.net *.split]
yosafbridge has quit [*.net *.split]
drbobbeaty has quit [*.net *.split]
lucerne has quit [*.net *.split]
<enebo[m]>
so in theory many of those may not be needed at some point
<headius>
heh lots of them
<enebo[m]>
how many dups are during the processing of kwargs inside processing of kwargs?
<headius>
I have been patching them to keep keywords as keywords but it is messy
<headius>
I think this is worth pushing and discussing
<headius>
it is working but it is messy
<headius>
hey I finally have fewer failures than when I started
<headius>
yes in theory
<enebo[m]>
unless it is ruby2_keywords or something like that
<headius>
there are a lot of defensive copies that are not needed for literal kwargs
<enebo[m]>
it is too bad we are using the dup to continue and not preserving the original
<enebo[m]>
but I know one part of that is the to_hash stuff
<enebo[m]>
but that should largely go away
<enebo[m]>
headius: if things have advanced enough to push then you should
<headius>
I did not push to master because I'm just form-fitting this code and it will surely break other stuff
<headius>
I have to fetch the kid, have a look over that and I'll bbiab
<headius>
I wish our primary way to pass args was some structured value rather than IRubyObject[] because this array-munging is really messy and error-prone
<headius>
worth pointing out that this does not recognize ruby2_keywords yet, which would be another set of conditionals along the way
<headius>
embedding positional and keyword args in the same array leads to a lot of messy code