All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Brandon Williams" <bmwill@google.com>
Subject: Re: [RFC/PATCH 2/3] wildmatch: add interface for precompiling wildmatch() patterns
Date: Sat, 24 Jun 2017 12:59:51 +0200	[thread overview]
Message-ID: <87zicx1wig.fsf@gmail.com> (raw)
In-Reply-To: <xmqqshiq9naq.fsf@gitster.mtv.corp.google.com>


On Sat, Jun 24 2017, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> +struct wildmatch_compiled *wildmatch_compile(const char *pattern, unsigned int flags)
>> +{
>> +	struct wildmatch_compiled *code = xmalloc(sizeof(struct wildmatch_compiled));
>> +	code->pattern = xstrdup(pattern);
>> +	code->flags = flags;
>> +
>> +	return code;
>> +}
>> +
>> +int wildmatch_match(struct wildmatch_compiled *code, const char *text)
>> +{
>> +	return wildmatch(code->pattern, text, code->flags);
>> +}
>
> Is the far-in-the-future vision to make this the other way around?
> That is, this being scaffolding, wildmatch_match() which is supposed
> to be precompiled match actually uses wildmatch() as its underlying
> engine, but when a viable compilation machinery is plugged in, the
> wildmatch_match() that takes a precompiled pattern will call into
> the machinery to execute the compiled pattern, and wildmatch() will
> be reimplemented as "compile, call wildmatch_match() once and
> discard" sequence?

Exactly there would be no functional difference in the results, only
fixed overhead.

wildmatch() would be the one-off lazy interface and
wildmatch_{compile,match,free}(), just like how you can have a wrapper
function that calls regcomp() followed by regexec() & regfree(), but
it's better to structure your code so you're not compiling & freeing the
pattern all the time.

Right now of course there's no difference in the behavior, and a
trivially more overhead with the extra xstrdup() & free(), but I wanted
to split up the discussion of the semantics of the interface from any
actual behavior change in wildmatch() which would make use of it further
down the line.

> Otherwise I'd be worried about wildmatch() vs wildmatch_match()
> introducing subtle behaviour differences that leads to hard to debug
> problems.

  reply	other threads:[~2017-06-24 10:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-22 21:38 [PATCH 0/3] wildmatch refactoring Ævar Arnfjörð Bjarmason
2017-06-22 21:38 ` [PATCH 1/3] wildmatch: remove unused wildopts parameter Ævar Arnfjörð Bjarmason
2017-06-22 21:38 ` [RFC/PATCH 2/3] wildmatch: add interface for precompiling wildmatch() patterns Ævar Arnfjörð Bjarmason
2017-06-24  1:39   ` Junio C Hamano
2017-06-24 10:59     ` Ævar Arnfjörð Bjarmason [this message]
2017-06-24 18:13       ` Junio C Hamano
2017-06-22 21:38 ` [RFC/PATCH 3/3] wildmatch: make use of the " Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87zicx1wig.fsf@gmail.com \
    --to=avarab@gmail.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.