Bug #9344: warning origin incorrect with instance_eval

• Status: Closed
• Assignee: k_takata (Ken Takata)
• Target version: 2.2.0
• ruby -v: ruby 1.9.3p484 (2013-11-22) [i386-mingw32]
• Backport: 2.0.0: DONE, 2.1: DONE

[thyresias (Thierry Lambert)]

File 'test.rb' contains:

$-w = 2
code = <<-RUBY
  x = /]]/
  y = /[a-z]+*/
RUBY
instance_eval code, 'foo.rb'

The execution reports:

foo.rb:1: warning: regular expression has ']' without escape: /]]/
test.rb:6: warning: nested repeat operator + and * was replaced with '*': /[a-z]+*/

The last line should state 'foo.rb:2' instead of 'test.rb:6'.

The bug is also present in: ruby 2.0.0p353 (2013-11-22) [i386-mingw32]

[srawlins (Sam Rawlins)]

• File 9344.patch 9344.patch added

This also applies to redundant repeat operator: z = /[a-z]** foo/ because both these warnings use the older warning, onig_verb_warn. I think this warning method can be considered defunct. If we switch these two warnings to instead use onig_syntax_warn, the bug is fixed. These are also the last two references to onig_verb_warn, so the definition can be deleted as well.

Attached is a patch, although as I understand it, this will actually need to be applied to Onigmo [1], then merged into Ruby. So I've also duplicated this issue at Onigmo [2].

[1] https://github.com/k-takata/Onigmo
[2] https://github.com/k-takata/Onigmo/issues/31

[srawlins (Sam Rawlins)]

• File 9344-v2.patch 9344-v2.patch added

k-takata noted that onig_verb_warn and onig_set_verb_warn_func are public API, so we cannot delete them. Attached is a smaller patch, which is also open as a pull request at Onigmo: https://github.com/k-takata/Onigmo/pull/32

[nobu (Nobuyoshi Nakada)]

Rather I think onig_syntax_warn() should use onig_verb_warn instead of calling rb_warn() directly.
And another warning function with source file and line number, instead of rb_compile_warn().

[srawlins (Sam Rawlins)]

Oops, k-takata already accepted that patch into the GitHub repository [1]. But I think I agree with you, nobu-san, onig_verb_warn was a more advanced API. I can file another patch against k-takata/Onigmo.

[1] https://github.com/k-takata/Onigmo/pull/32

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)
• Category set to regexp
• Status changed from Open to Assigned
• Assignee set to k_takata (Ken Takata)
• Target version set to 2.2.0
• Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN to 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED

[k_takata (Ken Takata)]

• Status changed from Assigned to Closed

Onigmo 5.14.1 was merged with r46831.
Fix for this issue is included in Onigmo 5.14.1.

Rather I think onig_syntax_warn() should use onig_verb_warn instead of calling rb_warn() directly.
And another warning function with source file and line number, instead of rb_compile_warn().

If you send me another pull request, I will merge it.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED to 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: DONE

merge partially r46831 into ruby_2_1 branch at r47519.
The patch was derived from https://github.com/k-takata/Onigmo/commit/bdfc1997aa15b6baddaf9a482c6610b32504bd86

[usa (Usaku NAKAMURA)]

• Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: DONE to 2.0.0: DONE, 2.1: DONE

committed into ruby_2_0_0 at r47547.