Re: [Patch] Refractor Thompson matcher

2013-07-31 Thread Paolo Carlini
Hi, On 07/31/2013 05:43 AM, Tim Shen wrote: I found some other wired problems in that patch after committing. Probably need more time to work on it, so now I revert it :( in order to bootstrap successfully the patch I needed these additional changes. Please double check at your end, run the

Re: [Patch] Refractor Thompson matcher

2013-07-31 Thread Tim Shen
On Wed, Jul 31, 2013 at 8:18 PM, Paolo Carlini paolo.carl...@oracle.com wrote: in order to bootstrap successfully the patch I needed these additional changes. Please double check at your end, run the testsuite, and if everything goes well, please commit (again ;) the whole thing. Fully tested:

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Tim Shen
On Mon, Jul 29, 2013 at 4:26 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Ah, excellent. As usual, let's wait a day or so for further comments and then please commit the whole thing. Commited. -- Tim Shen

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Paolo Carlini
On 07/30/2013 02:07 PM, Tim Shen wrote: On Mon, Jul 29, 2013 at 4:26 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Ah, excellent. As usual, let's wait a day or so for further comments and then please commit the whole thing. Commited. The bootstrap is currently broken due to this commit.

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Paolo Carlini
.. no, too many changes, please simply revert the commit ASAP and next time please test more carefully before posting. Thanks, Paolo.

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Paolo Carlini
Hi again, I reverted the commit and tested that mainline is fine again. Just to clarify how we normally handle these issues in v3: *temporarily*, to avoid the linkage issues which broke the bootstrap today, all the non-template functions must be inline, even if large. In the past normally we

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Tim Shen
On Wed, Jul 31, 2013 at 6:10 AM, Paolo Carlini paolo.carl...@oracle.com wrote: I reverted the commit and tested that mainline is fine again. Sorry for the accident! Just to clarify how we normally handle these issues in v3: *temporarily*, to avoid the linkage issues which broke the bootstrap

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Tim Shen
On Wed, Jul 31, 2013 at 6:44 AM, Tim Shen timshe...@gmail.com wrote: On Wed, Jul 31, 2013 at 6:10 AM, Paolo Carlini paolo.carl...@oracle.com wrote: I reverted the commit and tested that mainline is fine again. Sorry for the accident! Just to clarify how we normally handle these issues in

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Paolo Carlini
Hi, On 07/31/2013 12:44 AM, Tim Shen wrote: I see. So I include regex in different files and then compile them together, it broke. I've make every non-templated function in this commit inline. Now it compiles. Sorry again for this commit. Did you actually bootstrap test the patch? Because you

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Tim Shen
On Wed, Jul 31, 2013 at 7:03 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Did you actually bootstrap test the patch? Because you didn't last time, right? I compiled it and run the 28_regex testsuite, nothing wrong happened because there're all single-file testcases in it; but I ignored

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Paolo Carlini
Hi, On 07/31/2013 01:11 AM, Tim Shen wrote: On Wed, Jul 31, 2013 at 7:03 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Did you actually bootstrap test the patch? Because you didn't last time, right? I compiled it and run the 28_regex testsuite, nothing wrong happened because there're all

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Tim Shen
On Wed, Jul 31, 2013 at 7:18 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Please post the complete patch you intend to commit. Part of the GCC policy is also that all the patches must be posted complete, exactly as would be committed upon approval. Here it is. -- Tim Shen bfs.patch

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Paolo Carlini
On 07/31/2013 01:26 AM, Tim Shen wrote: On Wed, Jul 31, 2013 at 7:18 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Please post the complete patch you intend to commit. Part of the GCC policy is also that all the patches must be posted complete, exactly as would be committed upon approval.

Re: [Patch] Refractor Thompson matcher

2013-07-30 Thread Tim Shen
I found some other wired problems in that patch after committing. Probably need more time to work on it, so now I revert it :( -- Tim Shen

Re: [Patch] Refractor Thompson matcher

2013-07-29 Thread Paolo Carlini
Hi, On 07/29/2013 02:06 AM, Tim Shen wrote: On Mon, Jul 29, 2013 at 1:08 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Oh well, thanks. But then I expect specific testcases to come with it, for the various values of the parameter, both the default and the other other values. Otherwise, the

Re: [Patch] Refractor Thompson matcher

2013-07-29 Thread Tim Shen
On Mon, Jul 29, 2013 at 4:26 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Minor nit: it's not clear to me why in the previous patch you added the includes of map and queue. I use them in regex_grep_matcher.h and regex_grep_matcher.tcc; Is include/std/regex the right place where I include

Re: [Patch] Refractor Thompson matcher

2013-07-29 Thread Paolo Carlini
On 07/29/2013 10:37 AM, Tim Shen wrote: On Mon, Jul 29, 2013 at 4:26 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Minor nit: it's not clear to me why in the previous patch you added the includes of map and queue. I use them in regex_grep_matcher.h and regex_grep_matcher.tcc; Is

Re: [Patch] Refractor Thompson matcher

2013-07-28 Thread Paolo Carlini
Hi, On 07/28/2013 06:13 AM, Tim Shen wrote: Refractor the whole Thompson matcher using the queue-based(BFS) Bellman-Ford algorithm. Fix the grouping problem. Refactor, refactoring, etc, no 'r'. If the grouping problem is now fixed, would it make sense to add corresponding testcases? Paolo.

Re: [Patch] Refractor Thompson matcher

2013-07-28 Thread Tim Shen
On Sun, Jul 28, 2013 at 5:12 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Refactor, refactoring, etc, no 'r'. Thanks :) If the grouping problem is now fixed, would it make sense to add corresponding testcases? They are already added by http://gcc.gnu.org/ml/gcc-cvs/2013-07/msg00643.html

Re: [Patch] Refractor Thompson matcher

2013-07-28 Thread Paolo Carlini
Hi, On 07/28/2013 12:18 PM, Tim Shen wrote: They are already added by http://gcc.gnu.org/ml/gcc-cvs/2013-07/msg00643.html (though I found the changelog entry used old file names, I'll fix it later). This time it's the BFS approach that can correctly handle the problem instead of the DFS one.

Re: [Patch] Refractor Thompson matcher

2013-07-28 Thread Tim Shen
On Sun, Jul 28, 2013 at 9:44 PM, Paolo Carlini paolo.carl...@oracle.com wrote: I see. I was wondering if in this development stage it would be convenient to have somewhere a parameter allowing to switch by hand such internal details, useful for testing purposes too. Eventually may or may not go

Re: [Patch] Refractor Thompson matcher

2013-07-28 Thread Paolo Carlini
On 07/28/2013 05:50 PM, Tim Shen wrote: On Sun, Jul 28, 2013 at 9:44 PM, Paolo Carlini paolo.carl...@oracle.com wrote: I see. I was wondering if in this development stage it would be convenient to have somewhere a parameter allowing to switch by hand such internal details, useful for testing

Re: [Patch] Refractor Thompson matcher

2013-07-28 Thread Tim Shen
On Mon, Jul 29, 2013 at 1:08 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Oh well, thanks. But then I expect specific testcases to come with it, for the various values of the parameter, both the default and the other other values. Otherwise, the idea isn't really immediately useful. See

[Patch] Refractor Thompson matcher

2013-07-27 Thread Tim Shen
Refractor the whole Thompson matcher using the queue-based(BFS) Bellman-Ford algorithm. Fix the grouping problem. The original implementation uses a stack, which possibly runs slower when deduplicating; and cannot handle gourping correctly. The patch may not be very clear, so here's the whole