Thank you for testing.

I pushed a slightly modified version of the last patch to the stable branch:
https://hg.savannah.gnu.org/hgweb/octave/rev/89cfd9dc69d8

Yes, that change works to clear the warnings as well. But I have only built and tested on Linux, not Windows.

@Markus: Yes, changing unique_ptr to shared_ptr also fixes the warnings:

diff -r 19f51c7fa341 liboctave/util/glob-match.h
--- a/liboctave/util/glob-match.h       Sun Dec 10 18:01:29 2023 +0100
+++ b/liboctave/util/glob-match.h       Mon Dec 11 07:45:28 2023 -0500
@@ -125,7 +125,7 @@ private:

   std::string m_pat;

-  std::unique_ptr<glob_match> m_glob;
+  std::shared_ptr<glob_match> m_glob;
 };

 #endif

Make check passes as before.

I think mmuetzel reported those new inv fails (on more than just windows) over at discourse.  see https://octave.discourse.group/t/test-suite-results-of-octave-9/5077/6

With Clang 16.0.6 on Linux, there are a large number of warnings like this, apparently because m_glob is a unique_ptr.

In file included from ../libinterp/corefcn/defun.h:35:
In file included from ../libinterp/corefcn/defun-int.h:33:
In file included from ../libinterp/octave-value/ov-builtin.h:35:
In file included from ../libinterp/octave-value/ov-fcn.h:39:
In file included from ../libinterp/corefcn/symscope.h:38:
../liboctave/util/glob-match.h:116:3: warning: explicitly defaulted copy constructor is implicitly deleted [-Wdefaulted-function-deleted]
  symbol_match (const symbol_match&) = default;
  ^
../liboctave/util/glob-match.h:128:31: note: copy constructor of 'symbol_match' is implicitly deleted because field 'm_glob' has a deleted copy constructor
  std::unique_ptr<glob_match> m_glob;
                              ^
/usr/include/c++/13.2.1/bits/unique_ptr.h:522:7: note: 'unique_ptr' has been explicitly marked deleted here
      unique_ptr(const unique_ptr&) = delete;
      ^
../liboctave/util/glob-match.h:116:40: note: replace 'default' with 'delete'
  symbol_match (const symbol_match&) = default;
                                       ^~~~~~~
                                       delete
../liboctave/util/glob-match.h:118:17: warning: explicitly defaulted copy assignment operator is implicitly deleted [-Wdefaulted-function-deleted]
  symbol_match& operator = (const symbol_match&) = default;
                ^
../liboctave/util/glob-match.h:128:31: note: copy assignment operator of 'symbol_match' is implicitly deleted because field 'm_glob' has a deleted copy assignment operator
  std::unique_ptr<glob_match> m_glob;
                              ^
/usr/include/c++/13.2.1/bits/unique_ptr.h:523:19: note: 'operator=' has been explicitly marked deleted here
      unique_ptr& operator=(const unique_ptr&) = delete;
                  ^
../liboctave/util/glob-match.h:118:52: note: replace 'default' with 'delete'
  symbol_match& operator = (const symbol_match&) = default;
                                                   ^~~~~~~
                                                   delete

Replacing `default` with `delete` in two places in liboctave/util/glob-match.h as the compiler says eliminates the warnings.

diff -r 19f51c7fa341 liboctave/util/glob-match.h
--- a/liboctave/util/glob-match.h       Sun Dec 10 18:01:29 2023 +0100
+++ b/liboctave/util/glob-match.h       Sun Dec 10 13:54:58 2023 -0500
@@ -113,9 +113,9 @@ public:

   symbol_match (const std::string& pattern);

-  symbol_match (const symbol_match&) = default;
+  symbol_match (const symbol_match&) = delete;

-  symbol_match& operator = (const symbol_match&) = default;
+  symbol_match& operator = (const symbol_match&) = delete;

   ~symbol_match () = default;

Pls verify if this change is OK across platforms.

Thank you for the continuous review. See the attached patch where  I tried to address your feedback. (I hope I got it all.)

Still no documentation changes.


(file #55421)

Thank you for the feedback. I'll try and update the patch.

Regarding the difference in pattern matching on Windows and on other platforms: Implementing the '*' and '?' wildcards was possible in a couple of lines. Implementing `[]` group and range matches from scratch (in a POSIX compatible way) is probably more tricky. See the complexity and performance of the implementation from gnulib. I was hoping we could avoid that complexity and use something simple but fast (given that this function will apparently be called thousands of times).

One option to have the same behavior on all platforms would be to use that manual matching function everywhere (not just on Windows).
But it might be that the implementation of fnmatch on Linux is actually more performant than the function that is added for Windows in this patch.
Alternatively: Is there a POSIX function that does efficient pattern matching with only '*' and '?' wildcard characters (no '[]' groups or ranges)?

The glob_match class has a member function `glob`. That member function doesn't make sense for the symbol_match class (imho).
I'm also unsure if we need the overloads for string_vector input or the second (optional) input argument for the constructors.
Other than that, I agree that it might be a good idea to keep the interfaces of the two classes similar and to move them to the same files in liboctave.

I'm attaching a patch with a next attempt.

It implements manual wildcard ('?' and '*') matching for Windows. It's still significantly faster than calling the gnulib function. (The main point of this change.)
I hope it does the job now. At least, `make check` shows no new errors for me, and some exploratory testing indicates that `clear` still does what it should for patterns with '?' or '*'.

The documentation change for `clear` is still missing.


(file #55418)

Just one more tic-toc post:
With Dmitri's hint I get:

v10.0.0:
Elapsed time is 3182.15 seconds.
ans = 2803.4

v9.0.0
Elapsed time is 1129.42 seconds.
ans = 780.72

which shows that tic-toc has no issues and Octave-for-Windows' slowness is well confirmed.

Running /usr/bin/time -v make check I see:

Command being timed: "make check"
        User time (seconds): 296.14
        System time (seconds): 53.60
        Percent of CPU this job got: 118%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 4:55.11
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 1069032
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 15
        Minor (reclaiming a frame) page faults: 14303738
        Voluntary context switches: 117257
        Involuntary context switches: 5267
        Swaps: 0
        File system inputs: 60168
        File system outputs: 95160
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0

So, tic-toc is consistent with wall time.
Also while running "make check" top shows cpu load ~60% on average (why?); it looks like cputime() takes this into account.

Dmitri.
--

BTW, is tic-toc called or reset somewhere in the test suite?

In "normal" use (outside test suite) its output seems quite reasonable to me.

@Mmuetzel:
That's the output I got.
The factor 3 slower I got with cputime() for Octave-10.0.0 looks to fit my experience (I was doing other things while Octave ploughed thru the tests).
Could multithreading influence tic-toc? (Ryzen 7 has 16 "CPU's").

What can I do to test tic-toc before submitting a bug report?

Hmmm, apologies I'm late to the party, but I can confirm this slowness since crossbuilding Octave-10.0.0 on Nov. 27 and then running the test suite in Windows 10 as follows:

t = cputime (); tic; __run_test_suite__ (); toc; cputime () - t

where I got the following results on my Ryzen 7 5800 laptop:

v10.0.0:
Elapsed time = 30.121 seconds.
ans = 2847.5

v.9.0.0 (built Nov 18):
Elapsed time = 23.1807 seconds.
ans = 816.06

I figured it would be a temporary issue so didn't react immediately (and I have little time these weeks) but it seems to be serious ...
BTW I have that 10.0.0 build installed at work and didn't note particularly slow performance there doing regular work (mostly data processing and graphics). That's on much slower (virtual) HW.

Shouldn't the constructors use "const std::string&" instead of "const std::string"?  If there is no difference now because of move semantics, then maybe we should decide what our preferred style is and try to ensure that all of Octave uses it consistently.

Are patterns like a[bcd] used anywhere in Octave or its test suite?

Otherwise, it seems reasonable to avoid the unnecessary locale issues.

Forgot to mention: The extreme slowdown when linking with the UCRT isn't limited to builds with MXE Octave, the same can be observed when building natively on Windows with MSYS2:
https://github.com/gnu-octave/octave/actions/runs/7076535934

Duration of `make check` when linking with MSVCRT: 11m 7s
Duration of `make check` when linking with UCRT: timeout after 1h 12s