This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch] Fix wrong backup variable initialization in regex


Hi,

On 10/03/2013 10:55 PM, Tim Shen wrote:
On Thu, Oct 3, 2013 at 4:39 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
Seems trivial enough, Ok.
Committed.

PS: I suppose isn't easy to prepare a testcase which would exercise those
lines?
Telling the truth, this bug *is* revealed by rerunning testcases with
the non-default DFS matcher. Since the other matcher(BFS) is the
default, it can be fully tested every time we run `make check`; this
time I force(by editting  code) the dispatcher to use DFS, and find
this mistake.

To test both of them, rewriting every testcase seems not a good idea.
So I edit dispatching code by hand and test the non-defuault matcher
every 2-3 commits. Do we have a better solution?
Before figuring out a fully general solution, I would anyway add a testcase which manually switches the executor and tests for the problem. More elegant would be a testcase which due to its nature automatically leads to an executor switch. Not sure if it's possible in this specific case.

Anyway, we have two executors for a reason, if I remember correctly: one is slower but necessary to handle backreferences. And the switch is automatic. Thus I don't think it's *very* urgent to test the executor handling br on regexes which do *not* have backreferences. But in principle yes, since we deliver two, and the users have a way to choose which one they want (X) we should add to the testsuite 2 copies of each test which doesn't use br and test both executors. We don't have to do it all at once, however, only when need arises, like today, decide a name for the subdirectories which host tests for the non-default executor, and create those subdirectories when need arises, start adding to them.

Paolo.

(X) By the way, is that documented *somewhere* outside the code itself? It should, because it's specific to our implementation. Also, what happens if the users try to use the executor which doesn't handle br with regexes which do have br? Do we provide a sensible diagnostic? Well, unless we want to keep secret - that is, undocumented - the switching facility, but I'm thinking that we should not. Or maybe we can defer the choice, document it only when we are *really* sure that both executors are interchangeable and enough bug free on the regexes they both can handle correctly.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]