April 2024 - ruby-core - ml.ruby-lang.org

[ruby-core:117566] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block
by byroot (Jean Boussier) 17 Apr '24

17 Apr '24

Issue #15554 has been updated by byroot (Jean Boussier). > doesn't show warning. You are right, I got confused a bit my a `method_missing`. Please ignore that remark about `...`. On another note, these warnings found another small issue in the Rails test suite: https://github.com/rails/rails/pull/51585/files ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-107964 * 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 0

[ruby-core:115911] [Ruby master Bug#20089] Fiber#kill transfers to root fiber
by rmosolgo (Robert Mosolgo) 17 Apr '24

17 Apr '24

Issue #20089 has been reported by rmosolgo (Robert Mosolgo). ---------------------------------------- Bug #20089: Fiber#kill transfers to root fiber https://bugs.ruby-lang.org/issues/20089 * Author: rmosolgo (Robert Mosolgo) * Status: Open * Priority: Normal * ruby -v: ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- I was hoping to use `Fiber#kill` to clean up formerly `.transfer`-d Fibers and work around https://bugs.ruby-lang.org/issues/20081, but I found that `Fiber#kill` has a similar control flow jump behavior. Is this on purpose, or a bug? Here's a script to test the behavior: ```ruby manager = Fiber.new do worker = Fiber.new do puts "2. Begin Worker" manager.transfer puts "This should never print -- killed" end puts "1. Transfer to Worker" worker.transfer puts "3. Killing Worker" worker.kill puts "4. Finished manager" end manager.transfer puts "5. Finished script" ``` I expected items `1` through `5` to be printed in order, but in fact, `4` is never printed: ``` $ ruby fiber_transfer_test.rb 1. Transfer to Worker 2. Begin Worker 3. Killing Worker 5. Finished script ``` It seems like `worker.kill` is transferring control to the top-level fiber instead of giving it back to `manager`. I also tried having the thread kill _itself_, hoping it would return to the fiber that originally `.transfer`ed to it, but it also seems to jump out: ```ruby manager = Fiber.new do worker = Fiber.new do puts "2. Begin Worker" manager.transfer Fiber.current.kill puts "This should never print -- killed" end puts "1. Transfer to Worker" worker.transfer puts "3. Killing Worker" worker.transfer puts "4. Finished manager" end manager.transfer puts "5. Finished script" ``` Prints: ``` 1. Transfer to Worker 2. Begin Worker 3. Killing Worker 5. Finished script ``` -- https://bugs.ruby-lang.org/

2 5

[ruby-core:117562] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block
by ko1 (Koichi Sasada) 17 Apr '24

17 Apr '24

Issue #15554 has been updated by ko1 (Koichi Sasada). byroot (Jean Boussier) wrote in #note-28: > - It generated some false positive because of `...` delegation for methods that don't accept a block. for examoles? ```ruby class C def bar = yield def foo(...) bar(...) end end C.new.foo{} ``` doesn't show warning. ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-107961 * 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 0

[ruby-core:117561] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block
by ko1 (Koichi Sasada) 17 Apr '24

17 Apr '24

Issue #15554 has been updated by ko1 (Koichi Sasada). Matz hesitates to force to put `&_` (or other tricks) for duck typing methods, so I try to skip warning if a `foo` uses block, any other `foo` doesn't warn this warning even if it doesn't use a block. ```ruby class C0 def f = yield end class C1 < C0 def f = nil end [C0, C1].f{ block } # do not warn on both calls ``` https://github.com/ruby/ruby/pull/10559 ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-107960 * 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 0

[ruby-core:117558] [Ruby master Feature#15554] warn/error passing a block to a method which never use a block
by byroot (Jean Boussier) 17 Apr '24

17 Apr '24

Issue #15554 has been updated by byroot (Jean Boussier). I worked on clearing these warnings from the Rails test suite: https://github.com/Shopify/rails/commit/640c7c751fa2b5d3d2e634685fbf0807c63… - It caught one big mistake in the test suite: https://github.com/Shopify/rails/commit/640c7c751fa2b5d3d2e634685fbf0807c63… - It generated a few dozen false positives because of duck-typing (e.g. `Object#try(arg, &block)` but `NilClass#try(arg)`. - It generated some false positive because of `...` delegation for methods that don't accept a block. So mixed results I guess. I'm extremely happy it caught this mistake, but the false positive rate is still a bit too high. `...` should definitely be ignored. ---------------------------------------- Feature #15554: warn/error passing a block to a method which never use a block https://bugs.ruby-lang.org/issues/15554#change-107956 * 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 0

[ruby-core:117495] [Ruby master Bug#20422] Bugfix release process
by masterleep2 (Bill Lipa) 17 Apr '24

17 Apr '24

Issue #20422 has been reported by masterleep2 (Bill Lipa). ---------------------------------------- Bug #20422: Bugfix release process https://bugs.ruby-lang.org/issues/20422 * Author: masterleep2 (Bill Lipa) * Status: Open * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- I was wondering if it would be possible to have more frequent bug fix releases of the latest stable Ruby. For example, if there was a 3.x.1 available two to three months after 3.x came out, it would make it more appealing to upgrade from a bug safety perspective. Right now there is no bug fix release for 3.3. I'm not sure what the release process is for bug fix releases - is there any information on this available? I couldn't find any, or any place other than this bug tracker where it seemed like this would be discussed. Sorry if this is the wrong place. -- https://bugs.ruby-lang.org/

5 5

[ruby-core:117225] [Ruby master Feature#20347] Separate docs task from all
by hsbt (Hiroshi SHIBATA) 17 Apr '24

17 Apr '24

Issue #20347 has been reported by hsbt (Hiroshi SHIBATA). ---------------------------------------- Feature #20347: Separate docs task from all https://bugs.ruby-lang.org/issues/20347 * Author: hsbt (Hiroshi SHIBATA) * Status: Assigned * Assignee: hsbt (Hiroshi SHIBATA) ---------------------------------------- I would like to migrate RDoc as bundled gems at Ruby 3.5. We need to handle `install-doc` and related task with that. I removed `docs` task from `all` and re-order `docs` task at `install-all`. https://github.com/ruby/ruby/pull/10282 It works with RDoc as bundled gems. We can keep current behavior with this migration. I hope to merge changes for `all` task and re-order `install-all` task at Ruby 3.4. https://github.com/ruby/ruby/pull/10282/commits/b160083175aed062c320b8d76ea… After that, the default `make` task will not generate rdoc. We need to add `make all docs` for that. -- https://bugs.ruby-lang.org/

1 1

[ruby-core:117523] [Ruby master Feature#20429] Emit a performance warning when specially optimized core methods are redefined
by byroot (Jean Boussier) 17 Apr '24

17 Apr '24

Issue #20429 has been reported by byroot (Jean Boussier). ---------------------------------------- Feature #20429: Emit a performance warning when specially optimized core methods are redefined https://bugs.ruby-lang.org/issues/20429 * Author: byroot (Jean Boussier) * Status: Open ---------------------------------------- MRI has a number of core methods with special handling in the interpreter and both JITs that if they are redefined negatively impact Ruby performance. The methods currently are (I may be missing a few): - `Integer`: `#+`, `#-`, `#*`, `#/`, `#%`, `#<`, `#>`, `#<=`, `#>=` - `Float`: `#+`, `#-`, `#*`, `#/`, `#<`, `#>`, `#<=`, `#>=` - `String`: `#freeze`, `#size`, `#length`, `#empty?`, `#+`, `#succ`, `#%`, `#-@` - `Array`: `#size`, `#length`, `#empty?`, `#hash` - `Array`: `#size`, `#length`, `#empty?`, `#[]`, `#[]=` -- https://bugs.ruby-lang.org/

3 2

[ruby-core:117546] [Ruby master Feature#6413] Make Dir.entries default to Dir.entries(Dir.pwd)
by matz (Yukihiro Matsumoto) 17 Apr '24

17 Apr '24

Issue #6413 has been updated by matz (Yukihiro Matsumoto). Status changed from Assigned to Rejected As a language with concurrency, I don't think depending on the current directory implicitly is a good idea. Matz. ---------------------------------------- Feature #6413: Make Dir.entries default to Dir.entries(Dir.pwd) https://bugs.ruby-lang.org/issues/6413#change-107940 * Author: shevegen (Robert A. Heiler) * Status: Rejected * Assignee: matz (Yukihiro Matsumoto) ---------------------------------------- Is there a reason why Dir.entries requires one argument? I think it would be easier and more convenient for the userruby if it would default to the current directory. Like so: Dir.entries(i = Dir.pwd) Or is there a reason why this is not done? -- https://bugs.ruby-lang.org/

1 0

[ruby-core:117530] [Ruby master Bug#20430] Ruby 3.3.0 build fail with make: *** [io_buffer.o] Error 1
by shubham_yadav (Shubham Yadav) 16 Apr '24

16 Apr '24

Issue #20430 has been reported by shubham_yadav (Shubham Yadav). ---------------------------------------- Bug #20430: Ruby 3.3.0 build fail with make: *** [io_buffer.o] Error 1 https://bugs.ruby-lang.org/issues/20430 * Author: shubham_yadav (Shubham Yadav) * Status: Open * ruby -v: 3.3.0 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- I'm trying to build ruby 3.3.0 with openssl 1.1.0. It fails with below error. My base ruby is 3.2.2. In file included from ./include/ruby/internal/arithmetic/char.h:23, from ./include/ruby/internal/arithmetic.h:24, from ./include/ruby/ruby.h:28, from ./include/ruby/encoding.h:19, from ./include/ruby/io.h:16, from io_buffer.c:9: ./include/ruby/internal/arithmetic/int.h: In function ‘rb_int2num_inline’: ./include/ruby/internal/arithmetic/int.h:241: warning: comparison is always true due to limited range of data type ./include/ruby/internal/arithmetic/int.h:241: warning: comparison is always true due to limited range of data type ./include/ruby/internal/arithmetic/int.h: In function ‘rb_uint2num_inline’: ./include/ruby/internal/arithmetic/int.h:256: warning: comparison is always true due to limited range of data type io_buffer.c: In function ‘io_buffer_read_U8’: io_buffer.c:1872: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c:1872: error: (Each undeclared identifier is reported only once io_buffer.c:1872: error: for each function it appears in.) io_buffer.c: In function ‘io_buffer_write_U8’: io_buffer.c:1872: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_S8’: io_buffer.c:1873: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_S8’: io_buffer.c:1873: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_u16’: io_buffer.c:1875: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_u16’: io_buffer.c:1875: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_U16’: io_buffer.c:1876: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_U16’: io_buffer.c:1876: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_s16’: io_buffer.c:1877: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_s16’: io_buffer.c:1877: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_S16’: io_buffer.c:1878: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_S16’: io_buffer.c:1878: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_u32’: io_buffer.c:1880: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_u32’: io_buffer.c:1880: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_U32’: io_buffer.c:1881: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_U32’: io_buffer.c:1881: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_s32’: io_buffer.c:1882: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_s32’: io_buffer.c:1882: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_S32’: io_buffer.c:1883: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_S32’: io_buffer.c:1883: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_u64’: io_buffer.c:1885: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_u64’: io_buffer.c:1885: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_U64’: io_buffer.c:1886: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_U64’: io_buffer.c:1886: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_s64’: io_buffer.c:1887: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_s64’: io_buffer.c:1887: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_S64’: io_buffer.c:1888: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_S64’: io_buffer.c:1888: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_f32’: io_buffer.c:1890: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_f32’: io_buffer.c:1890: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_F32’: io_buffer.c:1891: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_F32’: io_buffer.c:1891: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_f64’: io_buffer.c:1892: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_f64’: io_buffer.c:1892: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_read_F64’: io_buffer.c:1893: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘io_buffer_write_F64’: io_buffer.c:1893: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) io_buffer.c: In function ‘Init_IO_Buffer’: io_buffer.c:3702: error: ‘RB_IO_BUFFER_HOST_ENDIAN’ undeclared (first use in this function) At top level: cc1: warning: unrecognized command line option "-Wno-maybe-uninitialized" cc1: warning: unrecognized command line option "-Wno-tautological-compare" cc1: warning: unrecognized command line option "-Wno-self-assign" cc1: warning: unrecognized command line option "-Wno-parentheses-equality" cc1: warning: unrecognized command line option "-Wno-constant-logical-operand" cc1: warning: unrecognized command line option "-Wno-cast-function-type" make: *** [io_buffer.o] Error 1 -- https://bugs.ruby-lang.org/

2 1