Project

General

Profile

Actions

Bug #9344

closed

warning origin incorrect with instance_eval

Added by thyresias (Thierry Lambert) about 10 years ago. Updated over 9 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 1.9.3p484 (2013-11-22) [i386-mingw32]
[ruby-core:59468]

Description

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]


Files

9344.patch (3.55 KB) 9344.patch srawlins (Sam Rawlins), 03/04/2014 06:49 AM
9344-v2.patch (1.87 KB) 9344-v2.patch srawlins (Sam Rawlins), 03/04/2014 08:20 PM

Updated by srawlins (Sam Rawlins) about 10 years ago

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

Updated by srawlins (Sam Rawlins) about 10 years ago

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

Updated by nobu (Nobuyoshi Nakada) about 10 years ago

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().

Updated by srawlins (Sam Rawlins) about 10 years ago

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

Updated by nobu (Nobuyoshi Nakada) almost 10 years ago

  • 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

Updated by k_takata (Ken Takata) over 9 years ago

  • 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.

Updated by nagachika (Tomoyuki Chikanaga) over 9 years ago

  • 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

Updated by usa (Usaku NAKAMURA) over 9 years ago

  • 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.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0