Re: [elpa] externals/compat 6a60af22da: Optimize string-search

Re: [elpa] externals/compat 6a60af22da: Optimize string-search

[Philip Kaludercic]

ELPA Syncer <elpasync@gnu.org> writes:

> branch: externals/compat
> commit 6a60af22da1f1a7b08457a4016b9412a475afe4a
> Author: Daniel Mendler <mail@daniel-mendler.de>
> Commit: Daniel Mendler <mail@daniel-mendler.de>
>
>     Optimize string-search
> ---
>  compat-28.el | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/compat-28.el b/compat-28.el
> index fe69315522..b305ab2fff 100644
> --- a/compat-28.el
> +++ b/compat-28.el
> @@ -52,9 +52,8 @@ issues are inherited."
>    (when (and start-pos (or (< (length haystack) start-pos)
>                             (< start-pos 0)))
>      (signal 'args-out-of-range (list start-pos)))
> -  (save-match-data
> -    (let ((case-fold-search nil))
> -      (string-match (regexp-quote needle) haystack start-pos))))
> +  (let (case-fold-search)
> +    (string-match-p (regexp-quote needle) haystack start-pos)))
>  
>  (compat-defun length= (sequence length) ;; [[compat-tests:length=]]
>    "Returns non-nil if SEQUENCE has a length equal to LENGTH."

I don't believe this is a legal optimisation, even if the regular
expression is quoted.  Consider

(progn
  (string-match (rx (* "a") (group (* "b")) (* "a")) "abba")
  (match-data))
; => (0 4 1 3)

and

(progn
  (string-match (rx (* "a") (group (* "b")) (* "a")) "abba")
  (string-search "foo" "foobar")
  (match-data))
; => (0 4 1 3)

but

(progn
  (string-match (rx (* "a") (group (* "b")) (* "a")) "abba")
  (let ((case-fold-search nil))
    (string-match (regexp-quote "foo") "foobar" 0))
  (match-data))
; => (0 3)

Re: [elpa] externals/compat 6a60af22da: Optimize string-search

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Daniel Mendler <mail@daniel-mendler.de>
> Date: Fri, 11 Aug 2023 06:07:32 +0000
> 
> I don't believe this is a legal optimisation, even if the regular
> expression is quoted.

It is certainly "legal", in the sense that it breaks no laws.

The GNU Coding Standards frown on using "legal" or "illegal" for
anything that doesn't involve the laws and jurisprudence; we use
"valid" and "invalid" instead.

Re: [elpa] externals/compat 6a60af22da: Optimize string-search

[Daniel Mendler]

On 8/11/23 08:07, Philip Kaludercic wrote:
> ELPA Syncer <elpasync@gnu.org> writes:
> 
>> branch: externals/compat
>> commit 6a60af22da1f1a7b08457a4016b9412a475afe4a
>> Author: Daniel Mendler <mail@daniel-mendler.de>
>> Commit: Daniel Mendler <mail@daniel-mendler.de>
>>
>>     Optimize string-search
>> ---
>>  compat-28.el | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/compat-28.el b/compat-28.el
>> index fe69315522..b305ab2fff 100644
>> --- a/compat-28.el
>> +++ b/compat-28.el
>> @@ -52,9 +52,8 @@ issues are inherited."
>>    (when (and start-pos (or (< (length haystack) start-pos)
>>                             (< start-pos 0)))
>>      (signal 'args-out-of-range (list start-pos)))
>> -  (save-match-data
>> -    (let ((case-fold-search nil))
>> -      (string-match (regexp-quote needle) haystack start-pos))))
>> +  (let (case-fold-search)
>> +    (string-match-p (regexp-quote needle) haystack start-pos)))
>>  
>>  (compat-defun length= (sequence length) ;; [[compat-tests:length=]]
>>    "Returns non-nil if SEQUENCE has a length equal to LENGTH."
> 
> I don't believe this is a legal optimisation, even if the regular
> expression is quoted.  Consider

Hi Philip,

did you notice that I replaced `string-match' with `string-match-p'?

Daniel

Re: [elpa] externals/compat 6a60af22da: Optimize string-search

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: Daniel Mendler <mail@daniel-mendler.de>
>> Date: Fri, 11 Aug 2023 06:07:32 +0000
>> 
>> I don't believe this is a legal optimisation, even if the regular
>> expression is quoted.
>
> It is certainly "legal", in the sense that it breaks no laws.
>
> The GNU Coding Standards frown on using "legal" or "illegal" for
> anything that doesn't involve the laws and jurisprudence; we use
> "valid" and "invalid" instead.

Ok, I did not know that.  Thanks.

Re: [elpa] externals/compat 6a60af22da: Optimize string-search

[Philip Kaludercic]

Daniel Mendler <mail@daniel-mendler.de> writes:

> On 8/11/23 08:07, Philip Kaludercic wrote:
>> ELPA Syncer <elpasync@gnu.org> writes:
>> 
>>> branch: externals/compat
>>> commit 6a60af22da1f1a7b08457a4016b9412a475afe4a
>>> Author: Daniel Mendler <mail@daniel-mendler.de>
>>> Commit: Daniel Mendler <mail@daniel-mendler.de>
>>>
>>>     Optimize string-search
>>> ---
>>>  compat-28.el | 5 ++---
>>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/compat-28.el b/compat-28.el
>>> index fe69315522..b305ab2fff 100644
>>> --- a/compat-28.el
>>> +++ b/compat-28.el
>>> @@ -52,9 +52,8 @@ issues are inherited."
>>>    (when (and start-pos (or (< (length haystack) start-pos)
>>>                             (< start-pos 0)))
>>>      (signal 'args-out-of-range (list start-pos)))
>>> -  (save-match-data
>>> -    (let ((case-fold-search nil))
>>> -      (string-match (regexp-quote needle) haystack start-pos))))
>>> +  (let (case-fold-search)
>>> +    (string-match-p (regexp-quote needle) haystack start-pos)))
>>>  
>>>  (compat-defun length= (sequence length) ;; [[compat-tests:length=]]
>>>    "Returns non-nil if SEQUENCE has a length equal to LENGTH."
>> 
>> I don't believe this is a legal optimisation, even if the regular
>> expression is quoted.  Consider
>
> Hi Philip,
>
> did you notice that I replaced `string-match' with `string-match-p'?

No, I missed that.  Never mind then.

> Daniel