Skip to content

Conversation

@NWilson
Copy link
Member

@NWilson NWilson commented Nov 20, 2024

Fixes #541

Copy link
Collaborator

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@NWilson NWilson force-pushed the user/niwilson/op_fail-quantifier branch from 7c9bf02 to 7063183 Compare November 21, 2024 09:53
Comment on lines -8002 to -8117
#ifdef DEBUG_SHOW_PARSED
fprintf(stderr, "** Unrecognized parsed pattern item 0x%.8x\n", *pptr);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dunno why this DEBUG code was added, but why not keep it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced it with an UNREACHABLE assertion. The printf is currently disabled, so if the condition is hit, you won't see the logging. The assertion is nice and noisy, so will help development.

Copy link
Contributor

@carenas carenas Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if by disabled you mean that there is a commented out:

#define DEBUG_SHOW_PARSED 1

then that is by design, as those are usually enabled on demand when additional debugging is needed

I Presume that Philip (or whoever uses that) would rather have a printf than an abort though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an abort because it's unreachable code, which should always generate an error during development, not just when "DEBUG_SHOW_PARSED" has been set (which no-one is likely to set).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEBUG_SHOW_PARSED is an ancient debugging feature that I put in when I was adding the initial parsing pre-pass for 10.23 and I wanted to see what its output was. I didn't think it would be of general use afterwards which is why I didn't imiplement anything more elaborate.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some updates there but never actually tried it. Is there anybody using it?

Fixes PCRE2Project#541

Also add further debug assertions to internal error paths.
@NWilson NWilson force-pushed the user/niwilson/op_fail-quantifier branch from 7063183 to 4fc85b0 Compare November 22, 2024 09:47
@zherczeg zherczeg merged commit b920888 into PCRE2Project:master Nov 22, 2024
18 checks passed
@NWilson NWilson deleted the user/niwilson/op_fail-quantifier branch December 17, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(Maybe) remove handling of quantifiers after OP_FAIL

4 participants