Add unsupported error code for jit

[zherczeg]

No description provided.

[minipli-oss]

I'm just questioning myself if this change breaks existing applications because of the new error code? It did, at least, had an impact on the tests, so other projects might be impacted as well?

/me goes looking...

For example, GNU grep would be affected, as it explicitly tests the possible error codes:

  ec = pcre2_jit_compile (pc->cre, PCRE2_JIT_COMPLETE);
  if (ec && ec != PCRE2_ERROR_JIT_BADOPTION && ec != PCRE2_ERROR_NOMEMORY)
    die (EXIT_TROUBLE, 0, _("JIT internal error: %d"), ec);

With your change it would die() on certain patterns instead of falling back to intepreter mode.

Judging by previous arguments made by yourself and @PhilipHazel in #157, such an API-breaking change is stuff for a PCRE3 only.

[minipli-oss]

Make this "pattern not supported by JIT\0" to drop the superfluous "does" and capitalize JIT, as done elsewhere.

[zherczeg]

You are right, I need to read the documentation first:
https://www.pcre.org/current/doc/html/pcre2_jit_compile.html

"In particular, PCRE2_ERROR_JIT_BADOPTION is returned if JIT is not supported or if an unknown bit is set in options." - So I cannot change the badoption case (need to revert).

The code in grep is simply wrong, they also did not read the documentation. In general we are talking about pcre2 api compatilbity, not bug compatibility with existing applications.

[minipli-oss]

The code in git is simply wrong...

Which code in particular?

I'm asking as I recently made git somewhat aware of a non-functional JIT in PCRE2 (git/git@50b6ad5). But that was still not correct in your eyes?

[zherczeg]

Sorry, it is grep, not git. I have already fixed the comment.

[minipli-oss]

Ahh, saw your edit. So you meant grep, not git. Still, breaking grep won't be welcomed by many, I fear ;)

[zherczeg]

This change can wait, unless it is urgent for somebody. We can even wait until the next grep release.

[carenas]

note some documentation might need updating as well.

the implications for this change from 7a23313, for example, is likely to require changes in users of this API.

[zherczeg]

This patch is just exploring options. I have another idea: we could add flag to jit compile, which allows returning other error codes than nomemory / badoption. Will not make the code much more complex, and we have plenty of options. What do you think?

[PhilipHazel]

I think it's a good idea to require a new option flag to enable the new return codes. That way there is no danger of breaking existing code, and applications can update in their own time if they want to make use of the new codes.

[carenas]

The code in grep is simply wrong, they also did not read the documentation. In general we are talking about pcre2 api compatilbity, not bug compatibility with existing applications.

FWIW GNU grep has been fixed

https://git.savannah.gnu.org/cgit/grep.git/commit/src/pcresearch.c?id=6e7253de1dd443be5f498078e1807ddfa4d364de