- ruby-core - ml.ruby-lang.org

[ruby-core:117684] [Ruby master Bug#20452] Ruby 3.3 on Alpine Linux results in a relatively shallow SystemStackError exception

by Earlopain (A S)

Issue #20452 has been reported by Earlopain (A S). ---------------------------------------- Bug #20452: Ruby 3.3 on Alpine Linux results in a relatively shallow SystemStackError exception https://bugs.ruby-lang.org/issues/20452 * Author: Earlopain (A S) * Status: Open * ruby -v: 3.3.1 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- This is a redo of https://bugs.ruby-lang.org/issues/14387, reported against a non-eol version. The same issue still applies on recent rubies, though I personally only have tested 3.3 and 3.2: ```rb n = 100000 res = {} 1.upto(n).to_a.inject(res) do |r, i| r[i] = {} end def f(x) x.each_value { |v| f(v) } end f(res) ``` The patch from https://bugs.ruby-lang.org/issues/14387#note-13 still works, though as per https://bugs.ruby-lang.org/issues/14387#note-16 it can't be accepted as is. With the patch applied: ``` (irb):8:in `block in f': stack level too deep (SystemStackError) from (irb):8:in `each_value' from (irb):8:in `f' from (irb):8:in `block in f' from (irb):8:in `each_value' from (irb):8:in `f' from (irb):8:in `block in f' from (irb):8:in `each_value' from (irb):8:in `f' ... 11549 levels... from /usr/local/lib/ruby/gems/3.3.0/gems/irb-1.11.0/exe/irb:9:in `<top (required)>' from /usr/local/bin/irb:25:in `load' from /usr/local/bin/irb:25:in `<main>' ``` Without the patch: ``` (irb):8:in `each_value': stack level too deep (SystemStackError) from (irb):8:in `f' from (irb):8:in `block in f' from (irb):8:in `each_value' from (irb):8:in `f' from (irb):8:in `block in f' from (irb):8:in `each_value' from (irb):8:in `f' from (irb):8:in `block in f' ... 286 levels... from /usr/local/lib/ruby/gems/3.3.0/gems/irb-1.11.0/exe/irb:9:in `<top (required)>' from /usr/local/bin/irb:25:in `load' from /usr/local/bin/irb:25:in `<main>' ``` To reproduce, I have removed lines 102-105 from https://github.com/docker-library/ruby/blob/f5753434bb23041dd9913bb7b650e7b…. Check out the previous ticket for more information, there are a few conversations about possible solutions. -- https://bugs.ruby-lang.org/

1 week, 3 days

3
6
0 0

[ruby-core:117644] [Ruby master Feature#20443] Allow Major GC's to be disabled

by eightbitraptor (Matthew Valentine-House)

Issue #20443 has been reported by eightbitraptor (Matthew Valentine-House). ---------------------------------------- Feature #20443: Allow Major GC's to be disabled https://bugs.ruby-lang.org/issues/20443 * Author: eightbitraptor (Matthew Valentine-House) * Status: Open ---------------------------------------- [[Github PR #10598]](https://github.com/ruby/ruby/pull/10598) ## Background Ruby's GC running during Rails requests can have negative impacts on currently running requests, causing applications to have high tail-latency. A technique to mitigate this high tail-latency is Out-of-band GC (OOBGC). This is basically where the application is run with GC disabled, and then GC is explicitly started after each request, or when no requests are in progress. This can reduce the tail latency, but also introduces problems of its own. Long GC pauses after each request reduce throughput. This is more pronounced on threading servers like Puma because all the threads have to finish processing user requests and be "paused" before OOBGC can be triggered. This throughput decrease happens for a couple of reasons: 1. There are few heuristics available for users to determine when GC should run, this means that in OOBGC scenarios, it's possible that major GC's are being run more than necessary. 2. The lack of any GC during a request means that lots of garbage objects have been created and not cleaned up, so the process is using more memory than it should - requiring major GC's run as part of OOBGC to do more work and therefore take more time. This ticket attempts to address these issues by: 1. Provide `GC.disable_major` and its antonym `GC.enable_major` to disable and enable only major GC 2. Provide `GC.needs_major?` as a basic heuristic allowing users to tell when Ruby should run a Major GC. These ideas were originally proposed by @ko1 and @byroot in [this rails issue](https://github.com/rails/rails/issues/50449) Disabling GC major's would still allow minor GC's to run during the request, avoiding the ballooning memory usage caused by not running GC at all, and reducing the time that a major takes when we do run it, because the nursery objects have been cleaned up during the request already so there is less work for a major GC to do. This can be used in combination with `GC.needs_major?` to selectively run an OOBGC only when necessary ## Implementation This PR adds 3 new methods to the `GC` module - `GC.disable_major` This prevents major GC's from running automatically. It does not restrict minors. When `objspace->rgengc.need_major_gc` is set and a GC is run, instead of running a major, new heap pages will be allocated and a minor run instead. `objspace->rgengc.need_major_gc` will remain set until a major is manually run. If a major is not manually run then the process will eventually run out of memory. When major GC's are disabled, object promotion is disabled. That is, no objects will increment their ages during a minor GC. This is to attempt to minimise heap growth during the period between major GC's, by restricting the number of old-gen objects that will remain unconsidered by the GC until the next major. When `GC.start` is run, then major GC's will be enabled, a GC triggered with the options passed to `GC.start`, and then `disable_major` will be set to the state it was in before `GC.start` was called. - `GC.enable_major` This simply unsets the bit preventing major GC's. This will revert the GC to normal generational behaviour. Everything behaves as default again. - `GC.needs_major?` This exposes the value of `objspace->rgengc.need_major_gc` to the user level API. This is already exposed in `GC.latest_gc_info[:need_major_by]` but I felt that a simpler interface would make this easier to use and result in more readable code. eg. ```ruby out_of_band do GC.start if GC.needs_major? end ``` Because object aging is disabled when majors are disabled it is recommended to use this in conjunction with `Process.warmup`, which will prepare the heap by running a major GC, compacting the heap, and promoting every remaining object to old-gen. This ensures that minor GC's are running over the smallets possible set of young objects when `GC.disable_major` is true. ## Benchmarks We ran some tests in production on Shopify's core monolith over a weekend and found that: **Mean time spent in GC, as well as p99.9 and p99.99 GC times are all improved.** <img width="1000" alt="Screenshot 2024-04-22 at 16 41 49" src="https://github.com/ruby/ruby/assets/31869/6cff5b11-2e21-40c1-bb84-d994e0e17…"> **p99 GC time is slightly higher.** <img width="1000" alt="Screenshot 2024-04-22 at 16 44 55" src="https://github.com/ruby/ruby/assets/31869/dc645cbe-9495-46f0-8485-24e790c42…"> We're running far fewer OOBGC major GC's now that we have `GC.needs_major?` than we were before, and we believe that this is contributing to a slightly increased number of minor GC's. raising the p99 slightly. **App response times are all improved** We see a 9% reduction in average and p99 response times when compared against standard GC (4% p99.9 and p99.99). <img width="1000" alt="Screenshot 2024-04-22 at 16 55 53" src="https://github.com/ruby/ruby/assets/31869/8a80c102-1564-4bc9-ba44-6e9a8b85f…"> This drops slightly to an 8% reduction in average and p99 response times when compared against standard OOBGC (3.59 p99.9 and 4% p99.99) <img width="1000" alt="Screenshot 2024-04-22 at 16 56 10" src="https://github.com/ruby/ruby/assets/31869/1baef7ea-0155-4ff9-8ba4-a967b7574…"> -- https://bugs.ruby-lang.org/

1 week, 4 days

6
9
0 0

[ruby-core:117697] [Ruby master Bug#20453] Pointer being freed was not allocated in Regexp timeout

by dodecadaniel (Daniel Colson)

Issue #20453 has been reported by dodecadaniel (Daniel Colson). ---------------------------------------- Bug #20453: Pointer being freed was not allocated in Regexp timeout https://bugs.ruby-lang.org/issues/20453 * Author: dodecadaniel (Daniel Colson) * Status: Open * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- https://bugs.ruby-lang.org/issues/20228 frees `stk_base` to avoid a memory leak, but `stk_base` is sometimes stack allocated ([see `xalloca`](https://github.com/ruby/ruby/blob/dde99215f2bc60c22a00fc941ff7f714f011e920/regexec.c#L1177-L1181)). So the free only works if the regex stack grows enough that it needs to double ([see `xmalloc` and `xrealloc` in `stack_double`](https://github.com/ruby/ruby/blob/dde99215f2bc60c22a00fc941ff7f714f011e920/regexec.c#L1210-L1249). Reproduction: ```ruby Regexp.timeout = 0.001 /^(a*)x$/ =~ "a" * 1000000 + "x"' ``` I'll open a PR shortly. https://bugs.ruby-lang.org/issues/20228 was backported to 3.3.1, so this bug affects that version as well. -- https://bugs.ruby-lang.org/

1 week, 4 days

1
1
0 0

[ruby-core:116218] [Ruby master Bug#20186] Ripper does not generate AST for some keywords in Ruby 3.3

by kzkn (Kazuki Nishikawa)

Issue #20186 has been reported by kzkn (Kazuki Nishikawa). ---------------------------------------- Bug #20186: Ripper does not generate AST for some keywords in Ruby 3.3 https://bugs.ruby-lang.org/issues/20186 * Author: kzkn (Kazuki Nishikawa) * Status: Open * Priority: Normal * ruby -v: 3.3.0 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- Ripper does not generate AST for the below keyword: - break - next - redo - retry - yield Ruby 3.2 ``` $ ruby -v ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux] $ ruby -rripper -e "p Ripper.sexp('break')" [:program, [[:break, []]]] $ ruby -rripper -e "p Ripper.sexp('next')" [:program, [[:next, []]]] $ ruby -rripper -e "p Ripper.sexp('redo')" [:program, [[:redo]]] $ ruby -rripper -e "p Ripper.sexp('retry')" [:program, [[:retry]]] $ ruby -rripper -e "p Ripper.sexp('yield')" [:program, [[:yield0]]] ``` Ruby 3.3 ``` $ ruby -v ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux] $ ruby -rripper -e "p Ripper.sexp('break')" nil $ ruby -rripper -e "p Ripper.sexp('next')" nil $ ruby -rripper -e "p Ripper.sexp('redo')" nil $ ruby -rripper -e "p Ripper.sexp('retry')" nil $ ruby -rripper -e "p Ripper.sexp('yield')" nil ``` -- https://bugs.ruby-lang.org/

1 week, 4 days

2
1
0 0

[ruby-core:117535] [Ruby master Misc#20432] Proposal for workflow changes related to teeny releases

by ufuk (Ufuk Kayserilioglu)

Issue #20432 has been reported by ufuk (Ufuk Kayserilioglu). ---------------------------------------- Misc #20432: Proposal for workflow changes related to teeny releases https://bugs.ruby-lang.org/issues/20432 * Author: ufuk (Ufuk Kayserilioglu) * Status: Open ---------------------------------------- ## Aim - The cadence of CRuby stable releases is very important for how the project is perceived. Users of CRuby want to get more frequent releases with bug and security fixes, so that they feel confident that their projects and businesses continue running smoothly and safely. - More frequent releases make the community see that the CRuby project is active and thriving. It also allows people to keep upgrading to the latest versions of Ruby with newer features and better security. - The current cadence of teeny releases are causing concerns in the community, and there have been multiple examples of late releases in [3.0 series](https://www.reddit.com/r/ruby/comments/r14z39/ruby_303_released/), [3.1 and 3.2 series](https://www.reddit.com/r/ruby/comments/18n54es/why_have_they_not_up… and now [3.3 series](https://bugs.ruby-lang.org/issues/20085). This is creating confusion and doubt in our community and making the community to [ask for a change in bug fix release process](https://bugs.ruby-lang.org/issues/20422). - We all agree that release management is difficult and branch maintainers have a hard job. If we can be making their jobs a little bit easier, they will have more time and opportunities to make more frequent releases. - This proposal aims to improve some of the workflows around stable branch release management so that branch maintainers have the opportunity to make more frequent bug fix and security releases, thus, addressing some of the concerns from the community. ## Proposal(s) 1. A lot of the time of a release manager seems to be spent doing backports to the corresponding stable branch. The current workflow asks bug reports to populate the fields for which stable versions need the fix backported to, and release managers are responsible for keeping up with this list and creating backport commits on stable branches. This process is fragile, since the release managers are usually the people with the least amount of context on the particulars of a bug-fix and have to apply the same changes to a different branch and resolve any potential conflicts. This is a task best done by people who already have the most amount of context on the change: the original bugfix author. As a result, my suggestion is to make it mandatory for any change that needs to be backported to have backport PRs opened (or corresponding backport patches attached to the Redmine tickets) by the author for the relevant stable branches. That way the authors of changes will be responsible for making sure that the tests pass and the changes apply cleanly on older branches, allowing branch maintainers to come in and merge the PRs, freeing up their time and focus. Since backport PRs will be opened by authors of changes, but only merged by branch maintainers, this will still allow maintainers to control what goes into stable branches. 2. It also seems like branch maintainers are trying to synchronize releases across different Ruby versions, which is causing delays in getting teeny releases out of the door. In my opinion, each stable branch should be responsible for its own teeny releases and there should be no need to synchronize the cadence between different Ruby versions. This will mean less time waiting for other branch maintainers who might be busy with other life priorities, unblocking branch maintainers to be able to do teeny releases whenever it is convenient. 3. Finally, it is my observation that branch maintainers usually make teeny releases whenever there is a security incident that needs to be addressed. A security fix is most definitely a good reason to make a teeny release, but a teeny release should not wait for a security concern to be addressed. There are many other important bug fixes constantly being backported (or needing backports) such as fixes for crashes, fixes for memory retention/leaks or fixes for other behaviour regressions. It is important for end-users to have those addressed as timely as possible, so I would suggest that branch maintainers aim to make about 6-7 teeny releases every year (some of which will be security releases) in order to deliver value to end-users continuously. ## Conclusions Continuous integration and continuous delivery are one of the greatest changes that have happened in the software engineering sector in the recent decades. They allow us to shorten the time of delivery of new features to end-users and reduce the amount of work-in-progress that is waiting for come into the work, thus reducing waste. By adopting some of the workflow changes that I have suggested above, and maybe other changes that I might have missed, the CRuby core team has an opportunity to make a positive impact on all the users of the Ruby language and to increase the quality perception of the project as a result. For that reason, I hope my proposals are considered and implemented by the Core team. Thanks for your consideration. -- https://bugs.ruby-lang.org/

1 week, 4 days

3
2
0 0

[ruby-core:116687] [Ruby master Bug#20262] Regex mismatch between Ruby 3.2.2 and 3.3.0

by weilandia (Nick Weiland)

Issue #20262 has been reported by weilandia (Nick Weiland). ---------------------------------------- Bug #20262: Regex mismatch between Ruby 3.2.2 and 3.3.0 https://bugs.ruby-lang.org/issues/20262 * Author: weilandia (Nick Weiland) * Status: Open * Priority: Normal * ruby -v: 3.3.0 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- This might be a duplicate of https://bugs.ruby-lang.org/issues/20098, but I cannot make it match with the backref so maybe not. Below example matches in 3.2.2 but not in 3.3.0 ``` ruby str = "------------abcdefg------------#3895912" re = /()\1\b\w*[a-zA-Z-]*\d+[\w-]{3,}\w+\b/ re.match?(str) ``` -- https://bugs.ruby-lang.org/

1 week, 4 days

2
2
0 0

[ruby-core:117670] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block

by ko1 (Koichi Sasada)

Issue #15554 has been updated by ko1 (Koichi Sasada). Status changed from Closed to Assigned BTW on our application I found the following issue on RSpec tests on strict mode. ``` it ... do subject do ... end # this subject ignores the block end ``` This kind of issue can not be checked with current relax mode (because `subject{}` works outside of `it` block). I feel this kind of information is useful if developers have (1) effective warning filtering mechanism and (2) a tool to list the suspicious methods by analyzing the acquired warnings. For (1), at least application authors doesn't need gem's warning. For (2), false positive will be ignored easily (as https://bugs.ruby-lang.org/issues/15554#note-57). Also I'm afraid that if we introduce strict mode option (like environment variable), people can believe duck typing methods should make explicit for accepting block or not. It is what Matz does not like (*now*). ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-108080 * Author: ko1 (Koichi Sasada) * Status: Assigned * Assignee: matz (Yukihiro Matsumoto) ---------------------------------------- # Abstract Warn or raise an ArgumentError if block is passed to a method which does not use a block. In other words, detect "block user methods" implicitly and only "block user methods" can accept a block. # Background Sometimes, we pass a block to a method which ignores the passed block accidentally. ``` def my_open(name) open(name) end # user hopes it works as Kernel#open which invokes a block with opened file. my_open(name){|f| important_work_with f } # but simply ignored... ``` To solve this issue, this feature request propose showing warnings or raising an exception on such case. Last developer's meeting, matz proposed `&nil` which declares this method never receive a block. It is explicit, but it is tough to add this `&nil` parameter declaration to all of methods (do you want to add it to `def []=(i, e, &nil)`?). (I agree `&nil` is valuable on some situations) # Spec ## Define "use a block" methods We need to define which method accepts a block and which method does not. * (1) method has a block parameter (`&b`) * (2) method body has `yield' * (3) method body has `super` (ZSUPER in internal terminology) or `super(...)` * (4) method body has singleton method (optional) (1) and (2) is very clear. I need to explain about (3) and (4). (3). `super` (ZSUPER) passes all parameters as arguments. So there is no surprise that which can accept `block`. However `super(...)` also passes a block if no explicit block passing (like `super(){}` or `super(&b)`) are written. I'm not sure we need to continue this strange specification, but to keep compatibility depending this spec, I add this rule. (4). surprisingly, the following code invoke a block: ``` def foo class << Object.new yield end end foo{ p :ok } #=> :ok ``` I'm also not sure we need to keep this spec, but to allow this spec, I added (4) rule. Strictly speaking, it is not required, but we don't keep the link from singleton class ISeq to lexical parent iseq now, so I added it. ## Exceptional cases A method called by `super` doesn`t warn warning even if this method doesn't use a block. The rule (3) can pass blocks easily and there are many methods don`t use a block. So my patch ignores callings by `super`. ## corner cases There are several cases to use block without (1)-(4) rules. ### `Proc.new/proc/lambda` without a block Now it was deprecated in r66772 (commit:9f1fb0a17febc59356d58cef5e98db61a3c03550). Related discussion: [Bug #15539] ### `block_given?` `block_given?` expects block, but I believe we use it with `yield` or a block parameter. If you know the usecase without them, please tell us. ### `yield` in `eval` We can't know `yield` (or (3), (4) rule) in an `eval` evaluating string at calling time. ``` def foo eval('yield`) end foo{} # at calling time, # we can't know the method foo can accept a block or not. ``` So I added a warning to use `yield` in `eval` like that: `test.rb:4: warning: use yield in eval will not be supported in Ruby 3.` Workaround is use a block parameter explicitly. ``` def foo &b eval('b.call') end foo{ p :ok } ``` # Implementation Strategy is: * [compile time] introduce `iseq::has_yield` field and check it if the iseq (or child iseq) contains `yield` (or something) * [calling time] if block is given, check `iseq::has_yield` flag and show warning (or raise an exception) https://gist.github.com/ko1/c9148ad0224bf5befa3cc76ed2220c0b On this patch, now it raises an error to make it easy to detect. It is easy to switch to show the warning. # Evaluation and discussion I tried to avoid ruby's tests. https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786 Here is a patch. There are several patterns to avoid warnings. ## tests for `block_given?`, `Proc.new` (and similar) without block Add a dummy block parameter. It is test-specific issue. ## empty `each` Some tests add `each` methods do not `yield`, like: `def each; end`. Maybe test-specific issue, and adding a dummy block parameter. ## Subtyping / duck typing https://github.com/ruby/ruby/blob/c01a5ee85e2d6a7128cccafb143bfa694284ca87/… This `parse` method doesn't use `yield`, but other sub-type's `parse` methods use. ## `super` with `new` method https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786#file-tests-pat… This method override `Class#new` method and introduce a hook with block (yield a block in this hook code). https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package/tar_writer.rb#… In this method, call `super` and it also passing a block. However, called `initialize` doesn't use a block. ## Change robustness This change reduce robustness for API change. `Delegator` requires to support `__getobj__` for client classes. Now `__getobj__` should accept block but most of `__getobj__` clients do not call given block. https://github.com/ruby/ruby/blob/trunk/lib/delegate.rb#L80 This is because of delegator.rb's API change. https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786#file-tests-pat… Nobu says calling block is not required (ignoring a block is no problem) so it is not a bug for delegator client classes. ## Found issues. ``` [ 2945/20449] Rinda::TestRingServer#test_do_reply = 0.00 s 1) Error: Rinda::TestRingServer#test_do_reply: ArgumentError: passing block to the method "with_timeout" (defined at /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:787) is never used. /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:635:in `test_do_reply' [ 2946/20449] Rinda::TestRingServer#test_do_reply_local = 0.00 s 2) Error: Rinda::TestRingServer#test_do_reply_local: ArgumentError: passing block to the method "with_timeout" (defined at /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:787) is never used. /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:657:in `test_do_reply_local' [10024/20449] TestGemRequestSetGemDependencyAPI#test_platform_mswin = 0.01 s 3) Error: TestGemRequestSetGemDependencyAPI#test_platform_mswin: ArgumentError: passing block to the method "util_set_arch" (defined at /home/ko1/src/ruby/trunk/lib/rubygems/test_case.rb:1053) is never used. /home/ko1/src/ruby/trunk/test/rubygems/test_gem_request_set_gem_dependency_api.rb:655:in `test_platform_mswin' [10025/20449] TestGemRequestSetGemDependencyAPI#test_platforms = 0.01 s 4) Error: TestGemRequestSetGemDependencyAPI#test_platforms: ArgumentError: passing block to the method "util_set_arch" (defined at /home/ko1/src/ruby/trunk/lib/rubygems/test_case.rb:1053) is never used. /home/ko1/src/ruby/trunk/test/rubygems/test_gem_request_set_gem_dependency_api.rb:711:in `test_platforms' ``` These 4 detection show the problem. `with_timeout` method (used in Rinda test) and `util_set_arch` method (used in Rubygems test) simply ignore the given block. So these tests are simply ignored. I reported them. (https://github.com/rubygems/rubygems/issues/2601) ## raise an error or show a warning? At least, Ruby 2.7 should show warning for this kind of violation with `-w`. How about for Ruby3? -- https://bugs.ruby-lang.org/

1 week, 5 days

1
0
0 0

[ruby-core:117669] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block

by byroot (Jean Boussier)

Issue #15554 has been updated by byroot (Jean Boussier). > Acceptable for a codebase as big as Rails? IMO: - Rails isn't a big code base. Perhaps when compared to the average gem, but not when compared to the average **application** people work on. - Adding a warnings that most people will disregard because it throw too many false positive isn't helping. - Also consider all dependencies will throw warnings too, so when running this on an application with a substantial amount of dependencies, this will generate a TON of noise. I'm not hostile to declare ignored block, I think it's a pretty good idea, and to be honest it's even a bit weird that block parameters are implicit, when others aren't. But if that's the argument, then it would make more sense to me to just always require declaring it, regardless of whether `yield` is used or not. ```ruby >> def foo = yield >> method(:foo).parameters => [] ``` If you are trying to generate API docs or something along these lines, needed to parse the source or bytecode to figure out if a `yield` exist is a lot of extra work. But regardless of what is decided, I think doing it in stages would be preferable. As it stand I'm pretty happy with this new warnings, and I think if it's to be made more verbose, it could wait for Ruby 3.5. ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-108079 * Author: ko1 (Koichi Sasada) * Status: Closed * Assignee: matz (Yukihiro Matsumoto) ---------------------------------------- # Abstract Warn or raise an ArgumentError if block is passed to a method which does not use a block. In other words, detect "block user methods" implicitly and only "block user methods" can accept a block. # Background Sometimes, we pass a block to a method which ignores the passed block accidentally. ``` def my_open(name) open(name) end # user hopes it works as Kernel#open which invokes a block with opened file. my_open(name){|f| important_work_with f } # but simply ignored... ``` To solve this issue, this feature request propose showing warnings or raising an exception on such case. Last developer's meeting, matz proposed `&nil` which declares this method never receive a block. It is explicit, but it is tough to add this `&nil` parameter declaration to all of methods (do you want to add it to `def []=(i, e, &nil)`?). (I agree `&nil` is valuable on some situations) # Spec ## Define "use a block" methods We need to define which method accepts a block and which method does not. * (1) method has a block parameter (`&b`) * (2) method body has `yield' * (3) method body has `super` (ZSUPER in internal terminology) or `super(...)` * (4) method body has singleton method (optional) (1) and (2) is very clear. I need to explain about (3) and (4). (3). `super` (ZSUPER) passes all parameters as arguments. So there is no surprise that which can accept `block`. However `super(...)` also passes a block if no explicit block passing (like `super(){}` or `super(&b)`) are written. I'm not sure we need to continue this strange specification, but to keep compatibility depending this spec, I add this rule. (4). surprisingly, the following code invoke a block: ``` def foo class << Object.new yield end end foo{ p :ok } #=> :ok ``` I'm also not sure we need to keep this spec, but to allow this spec, I added (4) rule. Strictly speaking, it is not required, but we don't keep the link from singleton class ISeq to lexical parent iseq now, so I added it. ## Exceptional cases A method called by `super` doesn`t warn warning even if this method doesn't use a block. The rule (3) can pass blocks easily and there are many methods don`t use a block. So my patch ignores callings by `super`. ## corner cases There are several cases to use block without (1)-(4) rules. ### `Proc.new/proc/lambda` without a block Now it was deprecated in r66772 (commit:9f1fb0a17febc59356d58cef5e98db61a3c03550). Related discussion: [Bug #15539] ### `block_given?` `block_given?` expects block, but I believe we use it with `yield` or a block parameter. If you know the usecase without them, please tell us. ### `yield` in `eval` We can't know `yield` (or (3), (4) rule) in an `eval` evaluating string at calling time. ``` def foo eval('yield`) end foo{} # at calling time, # we can't know the method foo can accept a block or not. ``` So I added a warning to use `yield` in `eval` like that: `test.rb:4: warning: use yield in eval will not be supported in Ruby 3.` Workaround is use a block parameter explicitly. ``` def foo &b eval('b.call') end foo{ p :ok } ``` # Implementation Strategy is: * [compile time] introduce `iseq::has_yield` field and check it if the iseq (or child iseq) contains `yield` (or something) * [calling time] if block is given, check `iseq::has_yield` flag and show warning (or raise an exception) https://gist.github.com/ko1/c9148ad0224bf5befa3cc76ed2220c0b On this patch, now it raises an error to make it easy to detect. It is easy to switch to show the warning. # Evaluation and discussion I tried to avoid ruby's tests. https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786 Here is a patch. There are several patterns to avoid warnings. ## tests for `block_given?`, `Proc.new` (and similar) without block Add a dummy block parameter. It is test-specific issue. ## empty `each` Some tests add `each` methods do not `yield`, like: `def each; end`. Maybe test-specific issue, and adding a dummy block parameter. ## Subtyping / duck typing https://github.com/ruby/ruby/blob/c01a5ee85e2d6a7128cccafb143bfa694284ca87/… This `parse` method doesn't use `yield`, but other sub-type's `parse` methods use. ## `super` with `new` method https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786#file-tests-pat… This method override `Class#new` method and introduce a hook with block (yield a block in this hook code). https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package/tar_writer.rb#… In this method, call `super` and it also passing a block. However, called `initialize` doesn't use a block. ## Change robustness This change reduce robustness for API change. `Delegator` requires to support `__getobj__` for client classes. Now `__getobj__` should accept block but most of `__getobj__` clients do not call given block. https://github.com/ruby/ruby/blob/trunk/lib/delegate.rb#L80 This is because of delegator.rb's API change. https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786#file-tests-pat… Nobu says calling block is not required (ignoring a block is no problem) so it is not a bug for delegator client classes. ## Found issues. ``` [ 2945/20449] Rinda::TestRingServer#test_do_reply = 0.00 s 1) Error: Rinda::TestRingServer#test_do_reply: ArgumentError: passing block to the method "with_timeout" (defined at /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:787) is never used. /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:635:in `test_do_reply' [ 2946/20449] Rinda::TestRingServer#test_do_reply_local = 0.00 s 2) Error: Rinda::TestRingServer#test_do_reply_local: ArgumentError: passing block to the method "with_timeout" (defined at /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:787) is never used. /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:657:in `test_do_reply_local' [10024/20449] TestGemRequestSetGemDependencyAPI#test_platform_mswin = 0.01 s 3) Error: TestGemRequestSetGemDependencyAPI#test_platform_mswin: ArgumentError: passing block to the method "util_set_arch" (defined at /home/ko1/src/ruby/trunk/lib/rubygems/test_case.rb:1053) is never used. /home/ko1/src/ruby/trunk/test/rubygems/test_gem_request_set_gem_dependency_api.rb:655:in `test_platform_mswin' [10025/20449] TestGemRequestSetGemDependencyAPI#test_platforms = 0.01 s 4) Error: TestGemRequestSetGemDependencyAPI#test_platforms: ArgumentError: passing block to the method "util_set_arch" (defined at /home/ko1/src/ruby/trunk/lib/rubygems/test_case.rb:1053) is never used. /home/ko1/src/ruby/trunk/test/rubygems/test_gem_request_set_gem_dependency_api.rb:711:in `test_platforms' ``` These 4 detection show the problem. `with_timeout` method (used in Rinda test) and `util_set_arch` method (used in Rubygems test) simply ignore the given block. So these tests are simply ignored. I reported them. (https://github.com/rubygems/rubygems/issues/2601) ## raise an error or show a warning? At least, Ruby 2.7 should show warning for this kind of violation with `-w`. How about for Ruby3? -- https://bugs.ruby-lang.org/

1 week, 5 days

1
0
0 0

[ruby-core:117653] [Ruby master Bug#20446] OUtdated https://cache.ruby-lang.org/pub/ruby/index.txt

by vo.x (Vit Ondruch)

Issue #20446 has been reported by vo.x (Vit Ondruch). ---------------------------------------- Bug #20446: OUtdated https://cache.ruby-lang.org/pub/ruby/index.txt https://bugs.ruby-lang.org/issues/20446 * Author: vo.x (Vit Ondruch) * Status: Open * ruby -v: ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux] * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- Is the https://cache.ruby-lang.org/pub/ruby/index.txt the place, where all Ruby releases can be found? If yes, then can this file be update with Ruby 3.3.1, 3.2.4 etc? If no, can this file be removed to avoid confusion? -- https://bugs.ruby-lang.org/

1 week, 5 days

3
4
0 0

[ruby-core:117663] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block

by Dan0042 (Daniel DeLorme)

Issue #15554 has been updated by Dan0042 (Daniel DeLorme). byroot (Jean Boussier) wrote in #note-53: > @ko1 here's the result (I ran it on the 5 most important sub components) I think it's worth mentioning there's 67 warnings in that output but "only" 25 uniques. Acceptable for a codebase as big as Rails? In order to understand the false positives I had a look at the "add_bind" warnings. And at first I thought the ignored block was actually a bug! It took me half an hour of looking through the code to finally understand what it did and why it was ok to ignore the block. So while it's a false positive, this mirrors my experience in #note-45 that making the arguments more explicit would really help understanding of the code. my 2¢ ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-108071 * Author: ko1 (Koichi Sasada) * Status: Closed * Assignee: matz (Yukihiro Matsumoto) ---------------------------------------- # Abstract Warn or raise an ArgumentError if block is passed to a method which does not use a block. In other words, detect "block user methods" implicitly and only "block user methods" can accept a block. # Background Sometimes, we pass a block to a method which ignores the passed block accidentally. ``` def my_open(name) open(name) end # user hopes it works as Kernel#open which invokes a block with opened file. my_open(name){|f| important_work_with f } # but simply ignored... ``` To solve this issue, this feature request propose showing warnings or raising an exception on such case. Last developer's meeting, matz proposed `&nil` which declares this method never receive a block. It is explicit, but it is tough to add this `&nil` parameter declaration to all of methods (do you want to add it to `def []=(i, e, &nil)`?). (I agree `&nil` is valuable on some situations) # Spec ## Define "use a block" methods We need to define which method accepts a block and which method does not. * (1) method has a block parameter (`&b`) * (2) method body has `yield' * (3) method body has `super` (ZSUPER in internal terminology) or `super(...)` * (4) method body has singleton method (optional) (1) and (2) is very clear. I need to explain about (3) and (4). (3). `super` (ZSUPER) passes all parameters as arguments. So there is no surprise that which can accept `block`. However `super(...)` also passes a block if no explicit block passing (like `super(){}` or `super(&b)`) are written. I'm not sure we need to continue this strange specification, but to keep compatibility depending this spec, I add this rule. (4). surprisingly, the following code invoke a block: ``` def foo class << Object.new yield end end foo{ p :ok } #=> :ok ``` I'm also not sure we need to keep this spec, but to allow this spec, I added (4) rule. Strictly speaking, it is not required, but we don't keep the link from singleton class ISeq to lexical parent iseq now, so I added it. ## Exceptional cases A method called by `super` doesn`t warn warning even if this method doesn't use a block. The rule (3) can pass blocks easily and there are many methods don`t use a block. So my patch ignores callings by `super`. ## corner cases There are several cases to use block without (1)-(4) rules. ### `Proc.new/proc/lambda` without a block Now it was deprecated in r66772 (commit:9f1fb0a17febc59356d58cef5e98db61a3c03550). Related discussion: [Bug #15539] ### `block_given?` `block_given?` expects block, but I believe we use it with `yield` or a block parameter. If you know the usecase without them, please tell us. ### `yield` in `eval` We can't know `yield` (or (3), (4) rule) in an `eval` evaluating string at calling time. ``` def foo eval('yield`) end foo{} # at calling time, # we can't know the method foo can accept a block or not. ``` So I added a warning to use `yield` in `eval` like that: `test.rb:4: warning: use yield in eval will not be supported in Ruby 3.` Workaround is use a block parameter explicitly. ``` def foo &b eval('b.call') end foo{ p :ok } ``` # Implementation Strategy is: * [compile time] introduce `iseq::has_yield` field and check it if the iseq (or child iseq) contains `yield` (or something) * [calling time] if block is given, check `iseq::has_yield` flag and show warning (or raise an exception) https://gist.github.com/ko1/c9148ad0224bf5befa3cc76ed2220c0b On this patch, now it raises an error to make it easy to detect. It is easy to switch to show the warning. # Evaluation and discussion I tried to avoid ruby's tests. https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786 Here is a patch. There are several patterns to avoid warnings. ## tests for `block_given?`, `Proc.new` (and similar) without block Add a dummy block parameter. It is test-specific issue. ## empty `each` Some tests add `each` methods do not `yield`, like: `def each; end`. Maybe test-specific issue, and adding a dummy block parameter. ## Subtyping / duck typing https://github.com/ruby/ruby/blob/c01a5ee85e2d6a7128cccafb143bfa694284ca87/… This `parse` method doesn't use `yield`, but other sub-type's `parse` methods use. ## `super` with `new` method https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786#file-tests-pat… This method override `Class#new` method and introduce a hook with block (yield a block in this hook code). https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package/tar_writer.rb#… In this method, call `super` and it also passing a block. However, called `initialize` doesn't use a block. ## Change robustness This change reduce robustness for API change. `Delegator` requires to support `__getobj__` for client classes. Now `__getobj__` should accept block but most of `__getobj__` clients do not call given block. https://github.com/ruby/ruby/blob/trunk/lib/delegate.rb#L80 This is because of delegator.rb's API change. https://gist.github.com/ko1/37483e7940cdc4390bf8eb0001883786#file-tests-pat… Nobu says calling block is not required (ignoring a block is no problem) so it is not a bug for delegator client classes. ## Found issues. ``` [ 2945/20449] Rinda::TestRingServer#test_do_reply = 0.00 s 1) Error: Rinda::TestRingServer#test_do_reply: ArgumentError: passing block to the method "with_timeout" (defined at /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:787) is never used. /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:635:in `test_do_reply' [ 2946/20449] Rinda::TestRingServer#test_do_reply_local = 0.00 s 2) Error: Rinda::TestRingServer#test_do_reply_local: ArgumentError: passing block to the method "with_timeout" (defined at /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:787) is never used. /home/ko1/src/ruby/trunk/test/rinda/test_rinda.rb:657:in `test_do_reply_local' [10024/20449] TestGemRequestSetGemDependencyAPI#test_platform_mswin = 0.01 s 3) Error: TestGemRequestSetGemDependencyAPI#test_platform_mswin: ArgumentError: passing block to the method "util_set_arch" (defined at /home/ko1/src/ruby/trunk/lib/rubygems/test_case.rb:1053) is never used. /home/ko1/src/ruby/trunk/test/rubygems/test_gem_request_set_gem_dependency_api.rb:655:in `test_platform_mswin' [10025/20449] TestGemRequestSetGemDependencyAPI#test_platforms = 0.01 s 4) Error: TestGemRequestSetGemDependencyAPI#test_platforms: ArgumentError: passing block to the method "util_set_arch" (defined at /home/ko1/src/ruby/trunk/lib/rubygems/test_case.rb:1053) is never used. /home/ko1/src/ruby/trunk/test/rubygems/test_gem_request_set_gem_dependency_api.rb:711:in `test_platforms' ``` These 4 detection show the problem. `with_timeout` method (used in Rinda test) and `util_set_arch` method (used in Rubygems test) simply ignore the given block. So these tests are simply ignored. I reported them. (https://github.com/rubygems/rubygems/issues/2601) ## raise an error or show a warning? At least, Ruby 2.7 should show warning for this kind of violation with `-w`. How about for Ruby3? -- https://bugs.ruby-lang.org/

1 week, 5 days

1
0
0 0

2024

2023

2022

ruby-core ----- 2024 ----- May 2024 April 2024 March 2024 February 2024 January 2024 ----- 2023 ----- December 2023 November 2023 October 2023 September 2023 August 2023 July 2023 June 2023 May 2023 April 2023 March 2023 February 2023 January 2023 ----- 2022 ----- December 2022 November 2022

ruby-core