GC must not be triggered during callback of rb_class_foreach_subclass.
To prevent GC, we can not use rb_ary_push. Instead, this changeset calls
rb_class_foreach_subclass twice: first counts the subclasses, then
allocates a buffer (which may cause GC and reduce subclasses, but not
increase), and finally stores the subclasses to the buffer.
[Bug #18282] [Feature #14394]
Refinement#import_methods imports methods from modules.
Unlike Module#include, it copies methods and adds them into the refinement,
so the refinement is activated in the imported methods.
[Bug #17429] [ruby-core:101639]
Must not be a bad idea to improve documents. [ci skip]
In fact many functions declared in the header file are already
documented more or less. They were just copy & pasted, with applying
some style updates.
Must not be a bad idea to improve documents. [ci skip]
In fact many functions declared in the header file are already
documented more or less. They were just copy & pasted, with applying
some style updates.
This commits implements size classes in the GC for the Variable Width
Allocation feature. Unless `USE_RVARGC` compile flag is set, only a
single size class is created, maintaining current behaviour. See the
redmine ticket for more details.
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
This commits implements size classes in the GC for the Variable Width
Allocation feature. Unless `USE_RVARGC` compile flag is set, only a
single size class is created, maintaining current behaviour. See the
redmine ticket for more details.
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Redo of 34a2acdac788602c14bf05fb616215187badd504 and
931138b00696419945dc03e10f033b1f53cd50f3 which were reverted.
GitHub PR #4340.
This change implements a cache for class variables. Previously there was
no cache for cvars. Cvar access is slow due to needing to travel all the
way up th ancestor tree before returning the cvar value. The deeper the
ancestor tree the slower cvar access will be.
The benefits of the cache are more visible with a higher number of
included modules due to the way Ruby looks up class variables. The
benchmark here includes 26 modules and shows with the cache, this branch
is 6.5x faster when accessing class variables.
```
compare-ruby: ruby 3.1.0dev (2021-03-15T06:22:34Z master 9e5105c) [x86_64-darwin19]
built-ruby: ruby 3.1.0dev (2021-03-15T12:12:44Z add-cache-for-clas.. c6be009) [x86_64-darwin19]
| |compare-ruby|built-ruby|
|:--------|-----------:|---------:|
|vm_cvar | 5.681M| 36.980M|
| | -| 6.51x|
```
Benchmark.ips calling `ActiveRecord::Base.logger` from within a Rails
application. ActiveRecord::Base.logger has 71 ancestors. The more
ancestors a tree has, the more clear the speed increase. IE if Base had
only one ancestor we'd see no improvement. This benchmark is run on a
vanilla Rails application.
Benchmark code:
```ruby
require "benchmark/ips"
require_relative "config/environment"
Benchmark.ips do |x|
x.report "logger" do
ActiveRecord::Base.logger
end
end
```
Ruby 3.0 master / Rails 6.1:
```
Warming up --------------------------------------
logger 155.251k i/100ms
Calculating -------------------------------------
```
Ruby 3.0 with cvar cache / Rails 6.1:
```
Warming up --------------------------------------
logger 1.546M i/100ms
Calculating -------------------------------------
logger 14.857M (± 4.8%) i/s - 74.198M in 5.006202s
```
Lastly we ran a benchmark to demonstate the difference between master
and our cache when the number of modules increases. This benchmark
measures 1 ancestor, 30 ancestors, and 100 ancestors.
Ruby 3.0 master:
```
Warming up --------------------------------------
1 module 1.231M i/100ms
30 modules 432.020k i/100ms
100 modules 145.399k i/100ms
Calculating -------------------------------------
1 module 12.210M (± 2.1%) i/s - 61.553M in 5.043400s
30 modules 4.354M (± 2.7%) i/s - 22.033M in 5.063839s
100 modules 1.434M (± 2.9%) i/s - 7.270M in 5.072531s
Comparison:
1 module: 12209958.3 i/s
30 modules: 4354217.8 i/s - 2.80x (± 0.00) slower
100 modules: 1434447.3 i/s - 8.51x (± 0.00) slower
```
Ruby 3.0 with cvar cache:
```
Warming up --------------------------------------
1 module 1.641M i/100ms
30 modules 1.655M i/100ms
100 modules 1.620M i/100ms
Calculating -------------------------------------
1 module 16.279M (± 3.8%) i/s - 82.038M in 5.046923s
30 modules 15.891M (± 3.9%) i/s - 79.459M in 5.007958s
100 modules 16.087M (± 3.6%) i/s - 81.005M in 5.041931s
Comparison:
1 module: 16279458.0 i/s
100 modules: 16087484.6 i/s - same-ish: difference falls within error
30 modules: 15891406.2 i/s - same-ish: difference falls within error
```
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Instead of on read. Once it's in the inline cache we never have to make
one again. We want to eventually put the value into the cache, and the
best opportunity to do that is when you write the value.
This change implements a cache for class variables. Previously there was
no cache for cvars. Cvar access is slow due to needing to travel all the
way up th ancestor tree before returning the cvar value. The deeper the
ancestor tree the slower cvar access will be.
The benefits of the cache are more visible with a higher number of
included modules due to the way Ruby looks up class variables. The
benchmark here includes 26 modules and shows with the cache, this branch
is 6.5x faster when accessing class variables.
```
compare-ruby: ruby 3.1.0dev (2021-03-15T06:22:34Z master 9e5105ca45) [x86_64-darwin19]
built-ruby: ruby 3.1.0dev (2021-03-15T12:12:44Z add-cache-for-clas.. c6be0093ae) [x86_64-darwin19]
| |compare-ruby|built-ruby|
|:--------|-----------:|---------:|
|vm_cvar | 5.681M| 36.980M|
| | -| 6.51x|
```
Benchmark.ips calling `ActiveRecord::Base.logger` from within a Rails
application. ActiveRecord::Base.logger has 71 ancestors. The more
ancestors a tree has, the more clear the speed increase. IE if Base had
only one ancestor we'd see no improvement. This benchmark is run on a
vanilla Rails application.
Benchmark code:
```ruby
require "benchmark/ips"
require_relative "config/environment"
Benchmark.ips do |x|
x.report "logger" do
ActiveRecord::Base.logger
end
end
```
Ruby 3.0 master / Rails 6.1:
```
Warming up --------------------------------------
logger 155.251k i/100ms
Calculating -------------------------------------
```
Ruby 3.0 with cvar cache / Rails 6.1:
```
Warming up --------------------------------------
logger 1.546M i/100ms
Calculating -------------------------------------
logger 14.857M (± 4.8%) i/s - 74.198M in 5.006202s
```
Lastly we ran a benchmark to demonstate the difference between master
and our cache when the number of modules increases. This benchmark
measures 1 ancestor, 30 ancestors, and 100 ancestors.
Ruby 3.0 master:
```
Warming up --------------------------------------
1 module 1.231M i/100ms
30 modules 432.020k i/100ms
100 modules 145.399k i/100ms
Calculating -------------------------------------
1 module 12.210M (± 2.1%) i/s - 61.553M in 5.043400s
30 modules 4.354M (± 2.7%) i/s - 22.033M in 5.063839s
100 modules 1.434M (± 2.9%) i/s - 7.270M in 5.072531s
Comparison:
1 module: 12209958.3 i/s
30 modules: 4354217.8 i/s - 2.80x (± 0.00) slower
100 modules: 1434447.3 i/s - 8.51x (± 0.00) slower
```
Ruby 3.0 with cvar cache:
```
Warming up --------------------------------------
1 module 1.641M i/100ms
30 modules 1.655M i/100ms
100 modules 1.620M i/100ms
Calculating -------------------------------------
1 module 16.279M (± 3.8%) i/s - 82.038M in 5.046923s
30 modules 15.891M (± 3.9%) i/s - 79.459M in 5.007958s
100 modules 16.087M (± 3.6%) i/s - 81.005M in 5.041931s
Comparison:
1 module: 16279458.0 i/s
100 modules: 16087484.6 i/s - same-ish: difference falls within error
30 modules: 15891406.2 i/s - same-ish: difference falls within error
```
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
To invalidate some callable method entries, we replace the entry in the
class. Most types of method entries are on the method table of the
origin class, but refinement entries without an orig_me are housed in
the method table of the class itself. They are there because refinements
take priority over prepended methods.
By unconditionally inserting a copy of the refinement entry into the
origin class, clearing the method cache created situations where there
are refinement entry duplicates in the lookup chain, leading to infinite
loops and other problems.
Update the replacement logic to use the right class that houses the
method entry. Also, be more selective about cache invalidation when
moving refinement entries for prepend. This avoids calling
clear_method_cache_by_id_in_class() before refinement entries are in the
place it expects.
[Bug #17806]
In every caller of `rb_class_ivar_set` it checks for the `RCLASS_IV_TBL`
and then creates it if it doesn't exist. Instead of repeating this in
every caller, this can be done once in `rb_class_ivar_set`.
It's important to only make the origin when the prepend goes
through, as the precense of the origin informs whether to do an
origin backfill.
This plus 2d877327e fix [Bug #17590].
Check for cyclic prepend before making any changes. This requires
scanning the module ancestor chain twice, but in general modules
do not have large numbers of ancestors.
Previously, if a class included a module and then prepended the
same module, the prepend had no effect. This changes the behavior
so that the prepend has an effect unless the module is already
prepended the receiver.
While here, rename the origin_seen variable in include_modules_at,
since it is misleading. The variable tracks whether c has been seen,
not whether the origin of klass has been.
Fixes [Bug #17423]
cee02d754d resets pCMC and `me`
will be a invalidated and continuing the invalidated `me`,
it will break the data structure. This patch tris to clear
all methods of specified class before manipulating the `me`s.
[Issue #17417]
Module#include should only be able to insert modules after the origin,
otherwise it ends up working like Module#prepend.
This fixes the case where one of the modules in the included module
chain is included in a module that is already prepended to the receiver.
Fixes [Bug #7844]
Before this commit, `clone` gave different results depending on whether the original object
had an attached singleton class or not.
Consider the following setup:
```
class Foo; end
Foo.singleton_class.define_method(:foo) {}
obj = Foo.new
obj.singleton_class if $call_singleton
clone = obj.clone
```
When `$call_singleton = false`, neither `obj.singleton_class.singleton_class` nor
`clone.singleton_class.singleton_class` own any methods.
However, when `$call_singleton = true`, `clone.singleton_class.singleton_class` would own a copy of
`foo` from `Foo.singleton_class`, even though `obj.singleton_class.singleton_class` does not.
The latter case is unexpected and results in a visibly different clone, depending on if the original object
had an attached class or not.
Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
Before this commit, iclasses were "shady", or not protected by write
barriers. Because of that, the GC needs to spend more time marking these
objects than otherwise.
Applications that make heavy use of modules should see reduction in GC
time as they have a significant number of live iclasses on the heap.
- Put logic for iclass method table ownership into a function
- Remove calls to WB_UNPROTECT and insert write barriers for iclasses
This commit relies on the following invariant: for any non oirigin
iclass `I`, `RCLASS_M_TBL(I) == RCLASS_M_TBL(RBasic(I)->klass)`. This
invariant did not hold prior to 98286e9 for classes and modules that
have prepended modules.
[Feature #16984]
98286e9850 made it so that
`Module#include` allocates an origin iclass on each use. Since `include`
is widely used, the extra allocation can contribute significantly to
memory usage.
Instead of always allocating in anticipation of prepend, this change
takes a different approach. The new setup inserts a origin iclass into
the super chains of all the children of the module when prepend happens
for the first time.
rb_ensure_origin is made static again since now that adding an origin
now means walking over all usages, we want to limit the number of places
where we do it.
3556a834a2 added support for
Module#include to affect the iclasses of the module. It didn't add
support for Module#prepend because there were bugs in the object model
and GC at the time that prevented it. Those problems have been
addressed in ad729a1d11 and
98286e9850, and now adding support for
it is straightforward and does not break any tests or specs.
Fixes [Bug #9573]
This fixes various issues when a module is included in or prepended
to a module or class, and then refined, or refined and then included
or prepended to a module or class.
Implement by renaming ensure_origin to rb_ensure_origin, making it
non-static, and calling it when refining a module.
Fix Module#initialize_copy to handle origins correctly. Previously,
Module#initialize_copy did not handle origins correctly. For example,
this code:
```ruby
module B; end
class A
def b; 2 end
prepend B
end
a = A.dup.new
class A
def b; 1 end
end
p a.b
```
Printed 1 instead of 2. This is because the super chain for
a.singleton_class was:
```
a.singleton_class
A.dup
B(iclass)
B(iclass origin)
A(origin) # not A.dup(origin)
```
The B iclasses would not be modified, so the includer entry would be
still be set to A and not A.dup.
This modifies things so that if the class/module has an origin,
all iclasses between the class/module and the origin are duplicated
and have the correct includer entry set, and the correct origin
is created.
This requires other changes to make sure all tests still pass:
* rb_undef_methods_from doesn't automatically handle classes with
origins, so pass it the origin for Comparable when undefing
methods in Complex. This fixed a failure in the Complex tests.
* When adding a method, the method cache was not cleared
correctly if klass has an origin. Clear the method cache for
the klass before switching to the origin of klass. This fixed
failures in the autoload tests related to overridding require,
without breaking the optimization tests. Also clear the method
cache for both the module and origin when removing a method.
* Module#include? is fixed to skip origin iclasses.
* Refinements are fixed to use the origin class of the module that
has an origin.
* RCLASS_REFINED_BY_ANY is removed as it was only used in a single
place and is no longer needed.
* Marshal#dump is fixed to skip iclass origins.
* rb_method_entry_make is fixed to handled overridden optimized
methods for modules that have origins.
Fixes [Bug #16852]
If a module has an origin, and that module is included in another
module or class, previously the iclass created for the module had
an origin pointer to the module's origin instead of the iclass's
origin.
Setting the origin pointer correctly requires using a stack, since
the origin iclass is not created until after the iclass itself.
Use a hidden ruby array to implement that stack.
Correctly assigning the origin pointers in the iclass caused a
use-after-free in GC. If a module with an origin is included
in a class, the iclass shares a method table with the module
and the iclass origin shares a method table with module origin.
Mark iclass origin with a flag that notes that even though the
iclass is an origin, it shares a method table, so the method table
should not be garbage collected. The shared method table will be
garbage collected when the module origin is garbage collected.
I've tested that this does not introduce a memory leak.
This change caused a VM assertion failure, which was traced to callable
method entries using the incorrect defined_class. Update
rb_vm_check_redefinition_opt_method and find_defined_class_by_owner
to treat iclass origins different than class origins to avoid this
issue.
This also includes a fix for Module#included_modules to skip
iclasses with origins.
Fixes [Bug #16736]
If a module has an origin, and that module is included in another
module or class, previously the iclass created for the module had
an origin pointer to the module's origin instead of the iclass's
origin.
Setting the origin pointer correctly requires using a stack, since
the origin iclass is not created until after the iclass itself.
Use a hidden ruby array to implement that stack.
Correctly assigning the origin pointers in the iclass caused a
use-after-free in GC. If a module with an origin is included
in a class, the iclass shares a method table with the module
and the iclass origin shares a method table with module origin.
Mark iclass origin with a flag that notes that even though the
iclass is an origin, it shares a method table, so the method table
should not be garbage collected. The shared method table will be
garbage collected when the module origin is garbage collected.
I've tested that this does not introduce a memory leak.
This also includes a fix for Module#included_modules to skip
iclasses with origins.
Fixes [Bug #16736]
When calling Module#include, if the receiver is a module,
walk the subclasses list and include the argument module in each
iclass.
This does not affect Module#prepend, as fixing that is significantly
more involved.
Fixes [Bug #9573]
This patch contains several ideas:
(1) Disposable inline method cache (IMC) for race-free inline method cache
* Making call-cache (CC) as a RVALUE (GC target object) and allocate new
CC on cache miss.
* This technique allows race-free access from parallel processing
elements like RCU.
(2) Introduce per-Class method cache (pCMC)
* Instead of fixed-size global method cache (GMC), pCMC allows flexible
cache size.
* Caching CCs reduces CC allocation and allow sharing CC's fast-path
between same call-info (CI) call-sites.
(3) Invalidate an inline method cache by invalidating corresponding method
entries (MEs)
* Instead of using class serials, we set "invalidated" flag for method
entry itself to represent cache invalidation.
* Compare with using class serials, the impact of method modification
(add/overwrite/delete) is small.
* Updating class serials invalidate all method caches of the class and
sub-classes.
* Proposed approach only invalidate the method cache of only one ME.
See [Feature #16614] for more details.
1. By substituting `n_var` with its initializer, `0 < n_var` is
equivalent to `argc > argi + n_trail`.
2. As `argi` is non-negative, so `argi + n_trail >= n_trail`, and
the above expression is equivalent to `argc > n_trail`.
3. Therefore, `f_last` is always false, and `last_hash` is no
longer used.
http://ci.rvm.jp/results/trunk_gcc7@silicon-docker/2539622
```
/tmp/ruby/v2/src/trunk_gcc7/class.c: In function 'rb_scan_args_parse':
/tmp/ruby/v2/src/trunk_gcc7/class.c:1971:12: warning: unused variable 'tmp_buffer' [-Wunused-variable]
VALUE *tmp_buffer = arg->tmp_buffer;
^~~~~~~~~~
```
```
In file included from /tmp/ruby/v2/src/trunk_gcc7/vm_insnhelper.c:1895:0,
from /tmp/ruby/v2/src/trunk_gcc7/vm.c:349:
/tmp/ruby/v2/src/trunk_gcc7/vm_args.c:212:1: warning: 'args_stored_kw_argv_to_hash' defined but not used [-Wunused-function]
args_stored_kw_argv_to_hash(struct args_info *args)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
```
This removes the warnings added in 2.7, and changes the behavior
so that a final positional hash is not treated as keywords or
vice-versa.
To handle the arg_setup_block splat case correctly with keyword
arguments, we need to check if we are taking a keyword hash.
That case didn't have a test, but it affects real-world code,
so add a test for it.
This removes rb_empty_keyword_given_p() and related code, as
that is not needed in Ruby 3. The empty keyword case is the
same as the no keyword case in Ruby 3.
This changes rb_scan_args to implement keyword argument
separation for C functions when the : character is used.
For backwards compatibility, it returns a duped hash.
This is a bad idea for performance, but not duping the hash
breaks at least Enumerator::ArithmeticSequence#inspect.
Instead of having RB_PASS_CALLED_KEYWORDS be a number,
simplify the code by just making it be rb_keyword_given_p().
Saves comitters' daily life by avoid #include-ing everything from
internal.h to make each file do so instead. This would significantly
speed up incremental builds.
We take the following inclusion order in this changeset:
1. "ruby/config.h", where _GNU_SOURCE is defined (must be the very
first thing among everything).
2. RUBY_EXTCONF_H if any.
3. Standard C headers, sorted alphabetically.
4. Other system headers, maybe guarded by #ifdef
5. Everything else, sorted alphabetically.
Exceptions are those win32-related headers, which tend not be self-
containing (headers have inclusion order dependencies).
(old)
test.rb:4: warning: The last argument is used as the keyword parameter
test.rb:1: warning: for `foo' defined here; maybe ** should be added to the call?
(new)
test.rb:4: warning: The last argument is used as keyword parameters; maybe ** should be added to the call
test.rb:1: warning: The called method `foo' is defined here
This makes behavior the same as super in instance_eval in method
in class. The reason this wasn't implemented before is that
there is a check to determine if the self in the current context
is of the expected class, and a module itself can be included
in multiple classes, so it doesn't have an expected class.
Implementing this requires giving iclasses knowledge of which
class created them, so that super call in the module method
knows the expected class for super calls. This reference
is called includer, and should only be set for iclasses.
Note that the approach Ruby uses in this check is not robust. If
you instance_eval another object of the same class and call super,
instead of an TypeError, you get super called with the
instance_eval receiver instead of the method receiver. Truly
fixing super would require keeping a reference to the super object
(method receiver) in each frame where scope has changed, and using
that instead of current self when calling super.
Fixes [Bug #11636]
After the previous commit, this was still broken. The reason it
was broken is that a refined module that hasn't been prepended to
yet keeps the refined methods in the module's method table. When
prepending, the module's method table is moved to the origin
iclass, and then the refined methods are moved from the method
table to a new method table in the module itself.
Unfortunately, that means that if a class has included the module,
prepending breaks the refinements, because when the methods are
moved from the origin iclass method table to the module method
table, they are removed from the method table from the iclass
created when the module was included earlier.
Fix this by always creating an origin class when including a
module that has any refinements, even if the refinements are
not currently used. I wasn't sure the best way to do that.
The approach I choose was to use an object flag. The flag is
set on the module when Module#refine is called, and if the
flag is present when the module is included in another module
or class, an origin iclass is created for the module.
Fixes [Bug #13446]
This previously did not work, and the reason it did not work is
that:
1) Refining a module or class that prepends other modules places
the refinements in the class itself and not the origin iclass.
2) Inclusion of a module that prepends other modules skips the
module itself, including only iclasses for the prepended modules
and the origin iclass.
Those two behaviors combined meant that the method table for the
refined methods for the included module never ends up in the
method lookup chain for the class including the module.
Fix this by not skipping the module itself when the module is
included. This requires some code rearranging in
rb_include_class_new to make sure the correct method tables and
origin settings are used for the created iclass.
As origin iclasses shouldn't be exposed to Ruby, this also
requires skipping modules that have origin iclasses in
Module#ancestors (classes that have origin iclasses were already
skipped).
Fixes [Bug #16242]
This removes the related tests, and puts the related specs behind
version guards. This affects all code in lib, including some
libraries that may want to support older versions of Ruby.
This reverts commits: 10d6a3aca78ba48c1b85fba8627dc1dd883de5ba6c6a25feca167e6b48f17cb96d41a53207979278595b3c4fdd1521f7cf89c11c5e69accf336082033632a812c0f56506be0d86427a3219 .
The reason for the revert is that we observe ABA problem around
inline method cache. When a cache misshits, we search for a
method entry. And if the entry is identical to what was cached
before, we reuse the cache. But the commits we are reverting here
introduced situations where a method entry is freed, then the
identical memory region is used for another method entry. An
inline method cache cannot detect that ABA.
Here is a code that reproduce such situation:
```ruby
require 'prime'
class << Integer
alias org_sqrt sqrt
def sqrt(n)
raise
end
GC.stress = true
Prime.each(7*37){} rescue nil # <- Here we populate CC
class << Object.new; end
# These adjacent remove-then-alias maneuver
# frees a method entry, then immediately
# reuses it for another.
remove_method :sqrt
alias sqrt org_sqrt
end
Prime.each(7*37).to_a # <- SEGV
```
Now that we have eliminated most destructive operations over the
rb_method_entry_t / rb_callable_method_entry_t, let's make them
mostly immutabe and mark them const.
One exception is rb_export_method(), which destructively modifies
visibilities of method entries. I have left that operation as is
because I suspect that destructiveness is the nature of that
function.
Tired of rb_method_entry_create(..., rb_method_definition_create(
..., &(rb_method_foo_t) {...})) maneuver. Provide a function that
does the thing to reduce copy&paste.
The deleted function was to destructively overwrite existing method
entries, which is now considered to be a bad idea. Delete it, and
assign a newly created method entry instead.
Cfuncs that use rb_scan_args with the : entry suffer similar keyword
argument separation issues that Ruby methods suffer if the cfuncs
accept optional or variable arguments.
This makes the following changes to : handling.
* Treats as **kw, prompting keyword argument separation warnings
if called with a positional hash.
* Do not look for an option hash if empty keywords are provided.
For backwards compatibility, treat an empty keyword splat as a empty
mandatory positional hash argument, but emit a a warning, as this
behavior will be removed in Ruby 3. The argument number check
needs to be moved lower so it can correctly handle an empty
positional argument being added.
* If the last argument is nil and it is necessary to treat it as an option
hash in order to make sure all arguments are processed, continue to
treat the last argument as the option hash. Emit a warning in this case,
as this behavior will be removed in Ruby 3.
* If splitting the keyword hash into two hashes, issue a warning, as we
will not be splitting hashes in Ruby 3.
* If the keyword argument is required to fill a mandatory positional
argument, continue to do so, but emit a warning as this behavior will
be going away in Ruby 3.
* If keyword arguments are provided and the last argument is not a hash,
that indicates something wrong. This can happen if a cfunc is calling
rb_scan_args multiple times, and providing arguments that were not
passed to it from Ruby. Callers need to switch to the new
rb_scan_args_kw function, which allows passing of whether keywords
were provided.
This commit fixes all warnings caused by the changes above.
It switches some function calls to *_kw versions with appropriate
kw_splat flags. If delegating arguments, RB_PASS_CALLED_KEYWORDS
is used. If creating new arguments, RB_PASS_KEYWORDS is used if
the last argument is a hash to be treated as keywords.
In open_key_args in io.c, use rb_scan_args_kw.
In this case, the arguments provided come from another C
function, not Ruby. The last argument may or may not be a hash,
so we can't set keyword argument mode. However, if it is a
hash, we don't want to warn when treating it as keywords.
In Ruby files, make sure to appropriately use keyword splats
or literal keywords when calling Cfuncs that now issue keyword
argument separation warnings through rb_scan_args. Also, make
sure not to pass nil in place of an option hash.
Work around Kernel#warn warnings due to problems in the Rubygems
override of the method. There is an open pull request to fix
these issues in Rubygems, but part of the Rubygems tests for
their override fail on ruby-head due to rb_scan_args not
recognizing empty keyword splats, which this commit fixes.
Implementation wise, adding rb_scan_args_kw is kind of a pain,
because rb_scan_args takes a variable number of arguments.
In order to not duplicate all the code, the function internals need
to be split into two functions taking a va_list, and to avoid passing
in a ton of arguments, a single struct argument is used to handle
the variables previously local to the function.
This function was created as a variant of st_copy with firing write
barrier.
It should have more explicit name, such as st_copy_with_write_barrier.
But because it is used only for copying iv_tbl, so I rename it to
rb_iv_tbl_copy now. If we face other use case than iv_tbl, we may want
to rename it to more general name.