patchGNU Octave - Patches: patch #10295, Add test for regexp match on UTF-8...

 
 

patch #10295: Add test for regexp match on UTF-8 string

Submitter:  Rafael Laboissière <rlaboiss>
Submitted:  Tue 15 Nov 2022 05:12:07 PM UTC
   
 
Category:  Core : other Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Mon 21 Nov 2022 07:52:41 AM UTC, comment #4: 

The test suite passed on our CI with this new test.

Closing as done.

Markus Mützel <mmuetzel>
Group administrator
Sat 19 Nov 2022 01:01:55 PM UTC, comment #3: 

I pushed your patch with the modifications outlined in comment #1:
https://hg.savannah.gnu.org/hgweb/octave/rev/75cbfaf709cb

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Fri 18 Nov 2022 05:33:08 PM UTC, comment #2: 

Yes, it is probably a better idea passing a character vector instead of a UTF8-encoded string to regexp BIST.

As regards the move towards UTF-16, if this happens in the future, then we will probably have to "#define PCRE2_CODE_UNIT_WIDTH 16" and link against libpcre2-16.

Rafael Laboissière <rlaboiss>
Wed 16 Nov 2022 05:59:54 PM UTC, comment #1: 

Thank you for the test. Imho, a test like this is probably good to have. It might also make sense to include a character from outside the BMP.

However, I've always seen it as kind of an implementation detail that character arrays are encoded in UTF-8 in Octave. That implementation detail might change in the future. (Maybe, they'll be UTF-16 encoded like in Matlab. Not sure how likely that is though.)
I guess we could just change that BIST if that should be the case.

Next thing I'm not certain about: Are we sure that the test sources are always interpreted as UTF-8 (independent of, e.g., the system locale)?
It would probably be good if they were. But I don't know if that is the case currently.
One way around that might be to replace the literal string with the corresponding UTF-8 encoded byte sequence:

diff -r 9f4a9dd4a6ee -r a6a427632ab1 libinterp/corefcn/regexp.cc
--- a/libinterp/corefcn/regexp.cc        Sun Nov 13 13:00:16 2022 -0500
+++ b/libinterp/corefcn/regexp.cc        Tue Nov 15 12:25:40 2022 -0300
@@ -919,6 +919,9 @@
 %!assert (regexp ('abcabc', 'abc$'), 4)
 %!assert (regexp ('abcabc', '^abc$'), zeros (1,0))

+## UTF-8 test with character array "âé🙂ïõù"
+%!assert (regexp (char ([195, 162, 195, 169, 240, 159, 153, 130, 195, 175, 195, 181, 195, 185]), "."), [1, 3, 5, 9, 11, 13])
+
 %!test
 %! [s, e, te, m, t] = regexp (' No Match ', 'f(.*)uck');
 %! assert (s, zeros (1,0));


That's even more relying on the fact that character arrays in Octave are currently UTF-8 encoded. But the test relies on that anyway...

What do you think?

Markus Mützel <mmuetzel>
Group administrator
Tue 15 Nov 2022 05:12:07 PM UTC, original submission:  

The BIST added by the patch attached below exercises the regexp function for a subject string encoded in UTF-8. This BIST does not correspond to any bug found in the code, but I think it is a good idea to ensure that UTF-8 strings are properly handled, just in case there will be unexpected changes in the PCRE-related code.

Rafael Laboissière <rlaboiss>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #53961:  regexp-test-utf8.patch added by rlaboiss (831B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by rlaboiss (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-11-21 mmuetzel StatusReady For Test Done
        Open/ClosedOpen Closed
    2022-11-19 mmuetzel StatusNone Ready For Test
    2022-11-15 rlaboiss Attached File- Added regexp-test-utf8.patch, #53961

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code