ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
@ 2024-03-14  9:32 byroot (Jean Boussier) via ruby-core
  2024-03-14 14:43 ` [ruby-core:117173] " jeremyevans0 (Jeremy Evans) via ruby-core
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: byroot (Jean Boussier) via ruby-core @ 2024-03-14  9:32 UTC (permalink / raw)
  To: ruby-core; +Cc: byroot (Jean Boussier)

Issue #20335 has been reported by byroot (Jean Boussier).

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335

* Author: byroot (Jean Boussier)
* Status: Open
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117173] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
@ 2024-03-14 14:43 ` jeremyevans0 (Jeremy Evans) via ruby-core
  2024-03-14 14:52 ` [ruby-core:117174] " Eregon (Benoit Daloze) via ruby-core
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jeremyevans0 (Jeremy Evans) via ruby-core @ 2024-03-14 14:43 UTC (permalink / raw)
  To: ruby-core; +Cc: jeremyevans0 (Jeremy Evans)

Issue #20335 has been updated by jeremyevans0 (Jeremy Evans).


Seems fine to me.  Looking at the code, I don't think it would be difficult to implement.

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107263

* Author: byroot (Jean Boussier)
* Status: Open
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117174] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
  2024-03-14 14:43 ` [ruby-core:117173] " jeremyevans0 (Jeremy Evans) via ruby-core
@ 2024-03-14 14:52 ` Eregon (Benoit Daloze) via ruby-core
  2024-03-14 14:53 ` [ruby-core:117175] " Eregon (Benoit Daloze) via ruby-core
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eregon (Benoit Daloze) via ruby-core @ 2024-03-14 14:52 UTC (permalink / raw)
  To: ruby-core; +Cc: Eregon (Benoit Daloze)

Issue #20335 has been updated by Eregon (Benoit Daloze).


I think omitting the first N frames is useful.
However I see little reason to have a `length` or `Range`, one can just `break/return` out of the `Thread.each_caller_location`, and it seems pretty rare to need that.
So I think it's best to only add `start = 1` for now.

In fact the `length` argument is already an issue for `caller_locations`: if it's too high it causes `caller_locations` to return an empty Array instead of whatever is available:
```
irb(main):001:0> caller_locations(10)
=> 
["/home/eregon/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb/ruby-lex.rb:248:in `catch'",
 "/home/eregon/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb/ruby-lex.rb:248:in `each_top_level_statement'",
 "/home/eregon/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb.rb:566:in `eval_input'",
 "/home/eregon/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb.rb:500:in `block in run'",
 "/home/eregon/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb.rb:499:in `catch'",
 "/home/eregon/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb.rb:499:in `run'",
 "/home/eregon/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb.rb:421:in `start'",
 "/home/eregon/.rubies/ruby-3.2.2/lib/ruby/gems/3.2.0/gems/irb-1.6.2/exe/irb:11:in `<top (required)>'",
 "/home/eregon/.rubies/ruby-3.2.2/bin/irb:25:in `load'",
 "/home/eregon/.rubies/ruby-3.2.2/bin/irb:25:in `<main>'"]
irb(main):002:0> caller_locations(20)
=> []
```
That implies it needs to count how many frames are available, which is inefficient.
So we certainly wouldn't want that behavior for `each_caller_location` as it would ruin a significant part of its performance advantage.
(we should probably also fix this for `caller_locations`, but that should be a separate ticket)

Similarly the Range form is also problematic as it also requires iterating all the stack to find how many frames there are to know which frames `0..-2` should use.

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107264

* Author: byroot (Jean Boussier)
* Status: Open
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117175] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
  2024-03-14 14:43 ` [ruby-core:117173] " jeremyevans0 (Jeremy Evans) via ruby-core
  2024-03-14 14:52 ` [ruby-core:117174] " Eregon (Benoit Daloze) via ruby-core
@ 2024-03-14 14:53 ` Eregon (Benoit Daloze) via ruby-core
  2024-03-14 15:08 ` [ruby-core:117180] " byroot (Jean Boussier) via ruby-core
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eregon (Benoit Daloze) via ruby-core @ 2024-03-14 14:53 UTC (permalink / raw)
  To: ruby-core; +Cc: Eregon (Benoit Daloze)

Issue #20335 has been updated by Eregon (Benoit Daloze).


Ah I mistyped, `caller_locations(20)` is of course just setting start, `caller_locations(0, 20)` has no problem, I'll edit my post on Redmine to use the Range form, which is a problem.

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107265

* Author: byroot (Jean Boussier)
* Status: Open
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117180] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
                   ` (2 preceding siblings ...)
  2024-03-14 14:53 ` [ruby-core:117175] " Eregon (Benoit Daloze) via ruby-core
@ 2024-03-14 15:08 ` byroot (Jean Boussier) via ruby-core
  2024-03-14 15:15 ` [ruby-core:117182] " Eregon (Benoit Daloze) via ruby-core
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: byroot (Jean Boussier) via ruby-core @ 2024-03-14 15:08 UTC (permalink / raw)
  To: ruby-core; +Cc: byroot (Jean Boussier)

Issue #20335 has been updated by byroot (Jean Boussier).


> I think omitting the first N frames is useful.

Agreed, that's the only one I want. The reason I started the issue by asking for the same parameters is purely consistency.

Now, if there are valid reasons not to support some (like you exposed), I'm totally fine with deliberately not supporting them. But what I'm trying to say is we should start with the same feature set and then selectively remove the one that may be problematic.

So I suppose at that stage the proposal would be:

```ruby
Thread.each_caller_location(1) # skip first frame
Thread.each_caller_location(1, 20) # skip first frame, yield the next 20 frames
Thread.each_caller_location(1..20) # ArgumentError
```

Is that correct?

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107270

* Author: byroot (Jean Boussier)
* Status: Open
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117182] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
                   ` (3 preceding siblings ...)
  2024-03-14 15:08 ` [ruby-core:117180] " byroot (Jean Boussier) via ruby-core
@ 2024-03-14 15:15 ` Eregon (Benoit Daloze) via ruby-core
  2024-04-17  7:53 ` [ruby-core:117549] " nobu (Nobuyoshi Nakada) via ruby-core
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eregon (Benoit Daloze) via ruby-core @ 2024-03-14 15:15 UTC (permalink / raw)
  To: ruby-core; +Cc: Eregon (Benoit Daloze)

Issue #20335 has been updated by Eregon (Benoit Daloze).


Right, Range is problematic, and only Range with positive indices seems an weird restriction so I think better no Range.

`Thread.each_caller_location(1, 20)` I think there are basically no use cases for this, so I wouldn't add it to keep things simple.
If one wants to e.g. detect the boundary between an app and a gem they should look at locations, not hardcode some frame depth limit.
It's kind of the purpose of `Thread.each_caller_location` to be able to dynamically detect the limit, and then an extra Integer limit feels redundant.

Could you change your proposal to only add `start = 1`, unless you have/find some good use cases for `Thread.each_caller_location(1, 20)`?

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107272

* Author: byroot (Jean Boussier)
* Status: Open
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117549] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
                   ` (4 preceding siblings ...)
  2024-03-14 15:15 ` [ruby-core:117182] " Eregon (Benoit Daloze) via ruby-core
@ 2024-04-17  7:53 ` nobu (Nobuyoshi Nakada) via ruby-core
  2024-04-17  9:34 ` [ruby-core:117554] " matz (Yukihiro Matsumoto) via ruby-core
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: nobu (Nobuyoshi Nakada) via ruby-core @ 2024-04-17  7:53 UTC (permalink / raw)
  To: ruby-core; +Cc: nobu (Nobuyoshi Nakada)

Issue #20335 has been updated by nobu (Nobuyoshi Nakada).


https://github.com/ruby/ruby/pull/10554

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107943

* Author: byroot (Jean Boussier)
* Status: Open
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117554] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
                   ` (5 preceding siblings ...)
  2024-04-17  7:53 ` [ruby-core:117549] " nobu (Nobuyoshi Nakada) via ruby-core
@ 2024-04-17  9:34 ` matz (Yukihiro Matsumoto) via ruby-core
  2024-04-17 10:16 ` [ruby-core:117557] " mame (Yusuke Endoh) via ruby-core
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: matz (Yukihiro Matsumoto) via ruby-core @ 2024-04-17  9:34 UTC (permalink / raw)
  To: ruby-core; +Cc: matz (Yukihiro Matsumoto)

Issue #20335 has been updated by matz (Yukihiro Matsumoto).


Accepted. Thank you.

Matz.


----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107949

* Author: byroot (Jean Boussier)
* Status: Open
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117557] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
                   ` (6 preceding siblings ...)
  2024-04-17  9:34 ` [ruby-core:117554] " matz (Yukihiro Matsumoto) via ruby-core
@ 2024-04-17 10:16 ` mame (Yusuke Endoh) via ruby-core
  2024-04-17 10:25 ` [ruby-core:117559] " Eregon (Benoit Daloze) via ruby-core
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mame (Yusuke Endoh) via ruby-core @ 2024-04-17 10:16 UTC (permalink / raw)
  To: ruby-core; +Cc: mame (Yusuke Endoh)

Issue #20335 has been updated by mame (Yusuke Endoh).


A little addition: the dicussion about length and range was considered (not ignored), but @matz said he didn't see the need to make it inconsistent with Kernel#caller and Kernel#caller_locations in this case.

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107955

* Author: byroot (Jean Boussier)
* Status: Closed
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117559] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
                   ` (7 preceding siblings ...)
  2024-04-17 10:16 ` [ruby-core:117557] " mame (Yusuke Endoh) via ruby-core
@ 2024-04-17 10:25 ` Eregon (Benoit Daloze) via ruby-core
  2024-04-17 17:15 ` [ruby-core:117571] " byroot (Jean Boussier) via ruby-core
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eregon (Benoit Daloze) via ruby-core @ 2024-04-17 10:25 UTC (permalink / raw)
  To: ruby-core; +Cc: Eregon (Benoit Daloze)

Issue #20335 has been updated by Eregon (Benoit Daloze).


mame (Yusuke Endoh) wrote in #note-11:
> A little addition: the dicussion about length and range was considered (not ignored), but @matz said he didn't see the need to make it inconsistent with Kernel#caller and Kernel#caller_locations in this case.

Thank you for the precision.
I think it is likely other Ruby implementations raise ArgumentError if the Range begin or end is negative, because it makes no sense (IOW wrong usage) to use that with `Thread.each_caller_location`:
it forces to walk the entire stack to count the number of frames when the point of `Thread.each_caller_location` is to not walk the entire stack but just the parts being yielded to the block.
I would suggest CRuby does the same for consistency.

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107957

* Author: byroot (Jean Boussier)
* Status: Closed
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117571] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
                   ` (8 preceding siblings ...)
  2024-04-17 10:25 ` [ruby-core:117559] " Eregon (Benoit Daloze) via ruby-core
@ 2024-04-17 17:15 ` byroot (Jean Boussier) via ruby-core
  2024-04-17 18:54 ` [ruby-core:117574] " Eregon (Benoit Daloze) via ruby-core
  2024-04-17 18:59 ` [ruby-core:117575] " byroot (Jean Boussier) via ruby-core
  11 siblings, 0 replies; 13+ messages in thread
From: byroot (Jean Boussier) via ruby-core @ 2024-04-17 17:15 UTC (permalink / raw)
  To: ruby-core; +Cc: byroot (Jean Boussier)

Issue #20335 has been updated by byroot (Jean Boussier).


> the point of Thread.each_caller_location is to not walk the entire stack 

Is it to not walk the entire stack, or simply not to generate the `Backtrace::Location` objects for the entire stack? My guess is that the later is most of the overhead, and the walking, while unfortunate, isn't that bad. But I haven't measured.

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107968

* Author: byroot (Jean Boussier)
* Status: Closed
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117574] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
                   ` (9 preceding siblings ...)
  2024-04-17 17:15 ` [ruby-core:117571] " byroot (Jean Boussier) via ruby-core
@ 2024-04-17 18:54 ` Eregon (Benoit Daloze) via ruby-core
  2024-04-17 18:59 ` [ruby-core:117575] " byroot (Jean Boussier) via ruby-core
  11 siblings, 0 replies; 13+ messages in thread
From: Eregon (Benoit Daloze) via ruby-core @ 2024-04-17 18:54 UTC (permalink / raw)
  To: ruby-core; +Cc: Eregon (Benoit Daloze)

Issue #20335 has been updated by Eregon (Benoit Daloze).


@byroot It's both, but for TruffleRuby I'm pretty sure the first matters much more.
These few allocations are no big deal on the JVM, so I would expect similar on JRuby.
Walking the stack is also obviously slower with a deeper stack (e.g. Rails), while the object allocations are pretty obvious and simply the number of times the block yielded before the user `break`s/`return`s.

I also don't see any valid use case for `Thread.each_caller_location` with a Range with negative begin or end, so that's why I think it's better to prevent it altogether instead of making it a performance trap (i.e. it's `O(stack depth)` instead of the expected `O(number of locations yielded)`).
(the only way to make that fast would be to maintain a stack depth field in every frame, but that's overhead in time and memory for every call just for this feature, which seems clearly unreasonable).

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107974

* Author: byroot (Jean Boussier)
* Status: Closed
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [ruby-core:117575] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
  2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
                   ` (10 preceding siblings ...)
  2024-04-17 18:54 ` [ruby-core:117574] " Eregon (Benoit Daloze) via ruby-core
@ 2024-04-17 18:59 ` byroot (Jean Boussier) via ruby-core
  11 siblings, 0 replies; 13+ messages in thread
From: byroot (Jean Boussier) via ruby-core @ 2024-04-17 18:59 UTC (permalink / raw)
  To: ruby-core; +Cc: byroot (Jean Boussier)

Issue #20335 has been updated by byroot (Jean Boussier).


> I also don't see any valid use case for Thread.each_caller_location with a Range with negative begin or end

It's still a slightly cheaper version of `caller_locations(range).each`. I agree that it's not super useful, but I don't think it hurts being there either, and I prefer consistency.

----------------------------------------
Feature #20335: `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations`
https://bugs.ruby-lang.org/issues/20335#change-107975

* Author: byroot (Jean Boussier)
* Status: Closed
----------------------------------------
`Thread.each_caller_location` was added to Ruby 3.2 as part of [Feature #16663] and is a very useful API for emitting warnings with a proper source location and similar use cases.

However in many of the cases where I used it, or seen it used, it was needed to skip the first, or a couple frames:

Examples:

Sorbet: https://github.com/Shopify/sorbet/blob/b27a14c247ace7cabdf0f348bfb11fdf0b7e9ab4/gems/sorbet-runtime/lib/types/private/caller_utils.rb#L6-L18

```ruby
    def self.find_caller
      skiped_first = false
      Thread.each_caller_location do |loc|
        unless skiped_first
          skiped_first = true
          next
        end

        next if loc.path&.start_with?("<internal:")

        return loc if yield(loc)
      end
      nil
    end
```

@fxn 's PR: https://github.com/ruby/ruby/blob/9c2e686719a5a4df5ea0b8a3b6a373ca6003c229/lib/bundled_gems.rb#L140-L146

```ruby
      frames_to_skip = 2
      location = nil
      Thread.each_caller_location do |cl|
        if frames_to_skip >= 1
          frames_to_skip -= 1
          next
        end
      # snipp...

```

### Proposal

I think it would be very useful if `Thread.each_caller_location` accepted the same arguments as `caller` and `caller_locations`:

```ruby
#each_caller_location(start = 1, length = nil)
#each_caller_location(range)
```

@jeremyevans0 what do you think?





-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-04-17 19:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-14  9:32 [ruby-core:117147] [Ruby master Feature#20335] `Thread.each_caller_location` should accept the same arguments as `caller` and `caller_locations` byroot (Jean Boussier) via ruby-core
2024-03-14 14:43 ` [ruby-core:117173] " jeremyevans0 (Jeremy Evans) via ruby-core
2024-03-14 14:52 ` [ruby-core:117174] " Eregon (Benoit Daloze) via ruby-core
2024-03-14 14:53 ` [ruby-core:117175] " Eregon (Benoit Daloze) via ruby-core
2024-03-14 15:08 ` [ruby-core:117180] " byroot (Jean Boussier) via ruby-core
2024-03-14 15:15 ` [ruby-core:117182] " Eregon (Benoit Daloze) via ruby-core
2024-04-17  7:53 ` [ruby-core:117549] " nobu (Nobuyoshi Nakada) via ruby-core
2024-04-17  9:34 ` [ruby-core:117554] " matz (Yukihiro Matsumoto) via ruby-core
2024-04-17 10:16 ` [ruby-core:117557] " mame (Yusuke Endoh) via ruby-core
2024-04-17 10:25 ` [ruby-core:117559] " Eregon (Benoit Daloze) via ruby-core
2024-04-17 17:15 ` [ruby-core:117571] " byroot (Jean Boussier) via ruby-core
2024-04-17 18:54 ` [ruby-core:117574] " Eregon (Benoit Daloze) via ruby-core
2024-04-17 18:59 ` [ruby-core:117575] " byroot (Jean Boussier) via ruby-core

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).