Re: Matching regex case-sensitively in C strings?

[Yuan Fu <casouri@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]



> On Nov 8, 2022, at 2:18 AM, Mattias Engdegård <mattiase@acm.org> wrote:
> 
> 7 nov. 2022 kl. 21.35 skrev Yuan Fu <casouri@gmail.com>:
> 
>> fast_c_string_match_ignore_case (Lisp_Object regexp,
>> 				 const char *string, ptrdiff_t len)
>> {
>> regexp = string_make_unibyte (regexp);
> 
> This is expensive and not obviously correct when it makes a difference. Ie, no longer "fast", and may hide bugs.
> Something should be done about that.
> 
>> // Why do we need to unwind stack?
>> specpdl_ref count = SPECPDL_INDEX ();
> 
> Because freeze_pattern pushes an unwind-protect on the specpdl.
> 
>> struct regexp_cache *cache_entry
>>   = compile_pattern (regexp, 0, Vascii_canon_table, 0, 0);
> 
> `Vascii_canon_table` is what makes it case-insensitive; you want to use Qnil (but you probably already know that now).
> Since this is the only thing that differs from your intended use, I suggest you generalise this subroutine with a boolean parameter.
> 
>> // What does freezing a pattern do?
>> freeze_pattern (cache_entry);
> 
> It locks the compiled pattern record to make the regexp engine reentrant (but here it also seems to be used for GC purposes; not sure about that).
> 
>> // What is re_match_object for? I see that it can be t, nil or a string.
>> re_match_object = Qt;
> 
> Described in regex-emacs.h:
> 
>> /* The string or buffer being matched.
>>   It is used for looking up syntax properties.
>> 
>>   If the value is a Lisp string object, match text in that string; if
>>   it's nil, match text in the current buffer; if it's t, match text
>>   in a C string.
>> 
>>   This value is effectively another parameter to re_search_2 and
>>   re_match_2.  No calls into Lisp or thread switches are allowed
>>   before setting re_match_object and calling into the regex search
>>   and match functions.  These functions capture the current value of
>>   re_match_object into gl_state on entry.
>> 

Thanks! How about:

Yuan


[-- Attachment #2: c_string_match.diff --]
[-- Type: application/octet-stream, Size: 2112 bytes --]

diff --git a/src/lisp.h b/src/lisp.h
index 1e41e2064c9..d0b3f8f05a5 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4772,6 +4772,8 @@ fast_string_match_ignore_case (Lisp_Object regexp, Lisp_Object string)
 
 extern ptrdiff_t fast_c_string_match_ignore_case (Lisp_Object, const char *,
 						  ptrdiff_t);
+extern ptrdiff_t fast_c_string_match (Lisp_Object, const char *, ptrdiff_t,
+				      bool);
 extern ptrdiff_t fast_looking_at (Lisp_Object, ptrdiff_t, ptrdiff_t,
                                   ptrdiff_t, ptrdiff_t, Lisp_Object);
 extern ptrdiff_t find_newline1 (ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t,
diff --git a/src/search.c b/src/search.c
index b5d6a442c0f..90856cf5c12 100644
--- a/src/search.c
+++ b/src/search.c
@@ -505,10 +505,29 @@ fast_string_match_internal (Lisp_Object regexp, Lisp_Object string,
 fast_c_string_match_ignore_case (Lisp_Object regexp,
 				 const char *string, ptrdiff_t len)
 {
+  return fast_c_string_match (regexp, string, len, true);
+}
+
+/* Match REGEXP against STRING, searching all of STRING and return the
+   index of the match, or negative on failure.  This does not clobber
+   the match data.  Ignore case when searching if IGNORE_CASE is true.
+
+   We assume that STRING contains single-byte characters.  */
+
+ptrdiff_t
+fast_c_string_match (Lisp_Object regepx,
+		     const char *string, ptrdiff_t len, bool ignore_case)
+{
+  /* FIXME: This is expensive and not obviously correct when it makes
+     a difference. I.e., no longer "fast", and may hide bugs.
+     Something should be done about this.  */
   regexp = string_make_unibyte (regexp);
+  /* Record specpdl index because freeze_pattern pushes an
+     unwind-protect on the specpdl.  */
   specpdl_ref count = SPECPDL_INDEX ();
+  Lisp_Object translate_table = ignore_case ? Vascii_canon_table : Qnil;
   struct regexp_cache *cache_entry
-    = compile_pattern (regexp, 0, Vascii_canon_table, 0, 0);
+    = compile_pattern (regexp, 0, translate_table, 0, 0);
   freeze_pattern (cache_entry);
   re_match_object = Qt;
   ptrdiff_t val = re_search (&cache_entry->buf, string, len, 0, len, 0);

[-- Attachment #3: Type: text/plain, Size: 2 bytes --]