Merged
Conversation
Contributor
Author
|
@kou error message |
kou
reviewed
Jul 12, 2022
return something at first Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
kou
reviewed
Aug 31, 2022
ext/strscan/strscan.c
Outdated
| int i = 0; | ||
| int z = 0; | ||
| VALUE captures = rb_hash_new(); | ||
| onig_foreach_name(RREGEXP(RMATCH(RREGEXP_PTR(pattern))->regexp)->ptr, i, z); |
Member
There was a problem hiding this comment.
Here is a template for the next step:
typedef struct {
struct strscanner *scanner;
VALUE captures;
} named_captures_data;
static int
named_captures_iter(const OnigUChar *name,
const OnigUChar *name_end,
int back_num,
int *back_refs,
OnigRegex regex,
void *arg)
{
named_captures_data *data = arg;
return 0;
}
/*
* call-seq:
* scanner.named_captures -> hash
*
*
*/
static VALUE
strscan_named_captures(VALUE self)
{
struct strscanner *p;
GET_SCANNER(self, p);
named_captures_data data;
data.scanner = p;
data.captures = rb_hash_new();
onig_foreach_name(RREGEXP_PTR(p->regex), named_captures_iter, &data);
return data.captures;
}I'll explain this later...
Member
There was a problem hiding this comment.
Sorry. I explain this in Japanese...
onig_foreach_name()の第2引数は各名前ごとに実行される関数を指定します。たとえば、/(?<a>.)(?<b>.)/ならaに対して1回、bに対して1回、計2回指定した関数が実行されます。第3引数はその関数が実行されるときにその関数にそのまま渡されます。↑のコードで言えば、第3引数に指定した&dataがnamed_captures_iter()の最後の引数void *argになります。
では、なぜこんなことをしないといけないのでしょうか。それは、Cにはクロージャー機能がないからです。
Rubyにはクロージャー機能があるので次のようにブロックの中からブロックの外にある変数にアクセスできます。
a = 1
[1, 2, 3].each do |x|
p [a, x]
endしかし、Cにはクロージャー機能がないのでブロックの外の変数にアクセスできません。ここで言えばnamed_captures_iter()の中から結果を入れるために用意したrb_hash_new()（VALUE capturesに入っているやつ）にはアクセスできません。そのため、関数内で使いたい値は明示的に渡さないといけません。そういうことができるようにするためにonig_foreach_name()の第3引数があります。
さらにしかし！今回は渡したい値が2つあります。struct strscanner *pとVALUE captures = rb_hash_new()です。あ、いや、strcut strscanner *pじゃだめだな。。。VALUE selfの方じゃないとだめだな。。。
さーせん、やっぱりこんな感じで。
typedef struct {
VALUE self;
VALUE captures;
} named_captures_data;
static int
named_captures_iter(const OnigUChar *name,
const OnigUChar *name_end,
int back_num,
int *back_refs,
OnigRegex regex,
void *arg)
{
named_captures_data *data = arg;
return 0;
}
/*
* call-seq:
* scanner.named_captures -> hash
*
*
*/
static VALUE
strscan_named_captures(VALUE self)
{
struct strscanner *p;
GET_SCANNER(self, p);
named_captures_data data;
data.self = self;
data.captures = rb_hash_new();
onig_foreach_name(RREGEXP_PTR(p->regex), named_captures_iter, &data);
return data.captures;
}で、複数の値を渡したいときは1つの値にしないといけないんです。そのためのCの機能がstructです。Rubyで言えばインスタンス変数だけがあるクラスみたいなもんです。structを使うと複数の値を1つにまとめることができます。なので、↓のようにすると1つの値にまとめることができます。
typedef struct {
VALUE self;
VALUE captures;
} named_captures_data;ということで、structを使ってselfとcapturesをnamed_captures_iter()に渡しています。
onig_foreach_name(RREGEXP_PTR(p->regex), named_captures_iter, &data);の&dataの&ってなに！？と思うと思いますが、ポインターの話になるので、今回はなにも考えずに&を使っておいてください。
で、この渡した値をどうやってnamed_captures_iter()で受け取るかと言うと
named_captures_data *data = arg;です。void *argなのでargはvoid *型なのですが、void *型は単なるポインター型です。一方、named_captures_data *もポインター型なのですが、named_captures_dataのポインター型です。void *型は任意のポインター型に変換できるので
named_captures_data *data = arg;で、named_captures_dataのポインターとして扱うよという意味になります。argがonig_foreach_name(RREGEXP_PTR(p->regex), named_captures_iter, &data);の&dataに相当するので、onig_foreach_name()に渡した値を元に戻しているくらいに思ってもらえれば十分です。
ここまでは、とりあえず、大丈夫ですか？
ここまでわかったら後はnamed_captures_iter()の中身を実装していくだけになります。
具体的にはこんな感じになるんですが、後でまた説明するので、とりあえず↑まででわからないところがないか確認してもらえますか！？
static int
named_captures_iter(const OnigUChar *name,
const OnigUChar *name_end,
int back_num,
int *back_refs,
OnigRegex regex,
void *arg)
{
named_captures_data *data = arg;
VALUE key = rb_str_new((const char *)name, name_end - name);
VALUE value = RUBY_Qnil;
int i;
for (i = 0; i < back_num; i++) {
value = strscan_aref(data->self, INT2NUM(back_refs[i]));
}
rb_hash_aset(data->captures, key, value);
return 0;
}
Member
|
A general note, in the StringScanner API almost everything of MatchData is exposed. Why not simply store a MatchData and use regular Regexp matching methods, and just delegate captures, named_captures etc to MatchData? It's what TruffleRuby does: https://github.com/oracle/truffleruby/blob/master/lib/truffle/strscan.rb |
Member
|
I don't know why we didn't use the approach because I'm not the original author. If there is no performance penalty with |
Member
Since strscan needs to remember capture groups it needs all the extra information from |
Member
Right. We can decide what should we do with a benchmarked result. |
kou
reviewed
Sep 13, 2022
test/strscan/test_stringscanner.rb
Outdated
| s = "foobarbaz" | ||
| re = /(?<f>foo)(?<r>bar)(?<z>baz)/ | ||
| scan = StringScanner.new(s) | ||
| scan.match? re |
Member
There was a problem hiding this comment.
We can use assert {...} for boolean expression to get better failure message:
Suggested change
| scan.match? re | |
| assert do | |
| scan.match?(re) | |
| end |
Contributor
Author
There was a problem hiding this comment.
Is it to be assert scan.match?(re) ?
Member
There was a problem hiding this comment.
In general, no. assert scan.match?(re) and assert {scan.match?(re)} is difference. And the latter is preferred for better message on failure. See also: https://github.com/ruby/power_assert
But we can't use assert {...} here. Sorry.
We can use assert {...} in ruby/strscan but we can't use assert {...} in ruby/ruby. Because ruby/ruby doesn't use test-unit gem.
So we should use assert_true(scan.match?(re)) here.
Contributor
Author
There was a problem hiding this comment.
I think that scan.match?(re) returns a length of matching. So can I change this test to check length of string?
Member
There was a problem hiding this comment.
Wow! I thought that it returns a boolean value because its name ends with ?.
You're right. We should use assert_equal(LENGTH, scan.match?(re)).
Contributor
Author
There was a problem hiding this comment.
I fixed this test.
fix: change doc style Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
fix: make simplify sample code Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
fix: make simple doc sample Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
fix: add space for arrow Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
fix: allow only for CRuby Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
…pks/strscan into fix-support-named-captures
kou
reviewed
Sep 18, 2022
kou
reviewed
Sep 18, 2022
Member
|
I don't know why system https://github.com/ruby/strscan/actions/runs/3076118365/jobs/4970052136#step:11:5 So I merge this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix #43