Re: [PATCH v2] grep: correctly identify utf-8 characters with \{b,w} in -P

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sun, Jan 08 2023, Carlo Marcelo Arenas Belón wrote:

> When UTF is enabled for a PCRE match, the corresponding flags are
> added to the pcre2_compile() call, but PCRE2_UCP wasn't included.
>
> This prevents extending the meaning of the character classes to
> include those new valid characters and therefore result in failed
> matches for expressions that rely on that extention, for ex:
>
>   $ git grep -P '\bÆvar'
>
> Add PCRE2_UCP so that \w will include Æ and therefore \b could
> correctly match the beginning of that word.
>
> This has an impact on performance that has been estimated to be
> between 20% to 40% and that is shown through the added performance
> test.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  grep.c                              |  2 +-
>  t/perf/p7822-grep-perl-character.sh | 42 +++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 1 deletion(-)
>  create mode 100755 t/perf/p7822-grep-perl-character.sh
>
> diff --git a/grep.c b/grep.c
> index 06eed69493..1687f65b64 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -293,7 +293,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
>  		options |= PCRE2_CASELESS;
>  	}
>  	if (!opt->ignore_locale && is_utf8_locale() && !literal)
> -		options |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);
> +		options |= (PCRE2_UTF | PCRE2_UCP | PCRE2_MATCH_INVALID_UTF);

I have a definite bias towards liking this change, it would help my find
myself :)

But I don't think it's safe to change the default behavior "git-grep",
it's not a mere bug fix, but a major behavior change for existing users
of grep.patternType=perl. E.g. on git.git:
	
	$ diff <(git -P grep -P '\d+') <(git -P grep -P '(*UCP)\d')
	53360a53361,53362
	> git-gui/po/ja.po:"- 第１行: 何をしたか、を１行で要約。\n"
	> git-gui/po/ja.po:"- 第２行: 空白\n"

So, it will help "do the right thing" on e.g. "\bÆ", but it will also
find e.g. CJK numeric characters for \d etc.

I see per the discussion on
https://github.com/PCRE2Project/pcre2/issues/185 and
https://lists.gnu.org/archive/html/bug-grep/2023-01/threads.html that
you submitted similar fixes to GNU grep & PCRE itself.

I see that GNU grep integrated it a couple of days ago as
https://git.savannah.gnu.org/cgit/grep.git/commit/?id=5e3b760f65f13856e5717e5b9d935f5b4a615be3

As most discussions about PCRE will eventually devolve into "what does
Perl do?": "Perl" itself will promiscuously use this behavior by
default.

E.g. here the same "１" character (not the ASCII digit "1") will be
matched from the command-line:

	$ perl -Mre=debug -CA -wE 'shift =~ /\d/' "１"
	Compiling REx "\d"
	Final program:
	   1: POSIXU[\d] (2)
	   2: END (0)
	stclass POSIXU[\d] minlen 1
	Matching REx "\d" against "%x{ff11}"
	UTF-8 string...
	Matching stclass POSIXU[\d] against "%x{ff11}" (3 bytes)
	   0 <> <%x{ff11}>           |   0| 1:POSIXU[\d](2)
	   3 <%x{ff11}> <>           |   0| 2:END(0)
	Match successful!
	Freeing REx: "\d"

But I don't think it makes sense for "git grep" (or GNU "grep") to
follow Perl in this particular case.

For those not familiar with its Unicode model it doesn't assume by
default that strings are Unicode, they have to be explicitly marked as
such. in the above example I'm declaring that all of "argv" is UTF-8
(via the "-CA" flag).

If I didn't supply that flag the string wouldn't have the UTF-8 flag,
and wouldn't match, as the Perl regex engine won't use Unicode semantics
except on Unicode target strings.

Even for Perl, this behavior has been troublesome. Opinions differ, but
I think many would agree (and I've CC'd the main authority on Perl's
regex engine) that doing this by default was *probably* a mistake.

You almost never want "everything Unicode considers a digit", and if you
do using e.g. \p{Nd} instead of \d would be better in terms of
expressing your intent. I see you're running into this on the PCRE
tracker, where you're suggesting that the equivalent of /a (or /aa)
would be needed.

	https://github.com/PCRE2Project/pcre2/issues/185#issuecomment-1374796393


Which brings me home to the seeming digression about "Perl"
above.

Unlike a programming language where you'll typically "mark" your data as
it comes in, natural text as UTF-8, binary data as such etc., a "grep"
utility has to operate on more of an "all or nothing" basis (except in
the case of "-a"). I.e. we're usually searching through unknown data.

Enabling this by default means that we'll pick up characters most people
probably wouldn't expect, particularly from near-binary data formats
(those that won't require "-a", but contain non-Unicode non-ASCII
sequences).

I don't have some completely holistic view of what we should do in every
case, e.g. we turned on PCRE2_UTF so that things like "-i" would Just
Work, but even case-insensitivity has its own unexpected edge cases in
Unicode.

But I don't think those edge cases are nearly as common as those we'd
run into by enabling PCRE2_UCP. Rather than trying to opt-out with "/a"
or "/aa" I think this should be opt-in.

As the example at the start shows you can already do this with "(*UCP)"
in the pattern, so perhaps we should just link to the pcre2pattern(3)
manual from git-grep(1)?