regcomp(3): Refactor and combine top-level expression parsers

[kevans]

Summary

The impending libregex will implement GNU extensions to bring BREs and
EREs closer together. Prepare for this and reduce the diff of libregex changes by
refactoring and combining the top-level parsers for EREs/BREs ahead of time.

Branching functionality has been split out to make it easier to follow the combined
version of the top-level parser. It may also be enabled in the parsing context to make
it easier when libregex enables branching for BREs.

A branching context was also added for the various branching functions and so that
BREs, for instance, can determine if they're the first expression in a chain of expressions
within the current branch and treat '*' as ordinary if so.

This should have no functional impact and negligible performance impact.

As an aside, I'm not against moving the pre- and post- parse logic in the top-level
parser (if (p->bre && EAT('^')) { ... } and if (p->bre && wasdollar) { ... }
out into their own functions, setting them as function pointers of the struct parse
in the caller of the top-level parser if (p->bre), and then just calling them
in the appropriate places if they're set. I just did not do this here because it
seemed like perhaps a step too far.

Test Plan

Run libc, sed, gsed, bsdgrep tests; exp-run if necessary

[cem]

I'm not familiar enough with grep to have any idea if these changes are correct.

If they pass the tests, though, I'm happy.

Some minor style nits below.

lib/libc/regex/regcomp.c

270 ↗

(On Diff #28828)

style nit: space between & and arguments

271–272 ↗

(On Diff #28828)

Seems like these should be bools instead?

293 ↗

(On Diff #28828)

same

519 ↗

(On Diff #28828)

style nit: return (nskip);

530 ↗

(On Diff #28828)

style nit: compare non-boolean values against 0 (rather than using !)

661–662 ↗

(On Diff #28828)

style nit: This should probably just be one line

[kevans]

lib/libc/regex/regcomp.c

271–272 ↗

(On Diff #28828)

I actually made them`bool` originally, then later changed them to int since I had no argument to introduce bool to a file that hasn't used them yet. I then forgot to correct all of the testing of them down below to use int-style rather than bool-style.

Admittedly, that's probably bad reasoning, so I'll go ahead and fix that.

[cem]

lib/libc/regex/regcomp.c

271–272 ↗

(On Diff #28828)

If the dominant pattern in the file is int-style boolean values, this is ok. (And probably better, just for consistency's sake.) I'm definitely less familiar with the file than you are, so your best judgement on it is fine with me.

(Even bool-style conditionals are probably fine, given they're standing in for boolean values.)

[kevans]

• Revise style based on recommendations by cem@, use bool appropriately
• Be more explicit about initialization
• Do previously mentioned refactoring of BOL/EOL checks for BREs out into BRE-specific pre-/post- parse functions

Precedent for int-style bool was not all that strong, and I think I touched most of the cases anyways, so I ran with it
and used bools for the new stuff again.

I don't think performance impact of the pre-/post- parse function stuff should be bad. EREs won't have them, so they
have no need to dereference+call. BREs only do one iteration of this outer loop per invocation, with one invocation for each
expression or (subexpression), so performance is not really degraded from this. Besides this, we do have a couple benefits:

• Slightly more readable top-level parser, removing bits that are specific to the one expression type
• Less frequent checking whether we're parsing a BRE or not; benefit likely mitigated by the dereference+call overhead

[pfg]

I like bools too :).

[kevans]

PIng @emaste =)

[emaste]

Presumably the tests on the base system tools all passed. Do you think it's worth requesting an exp-run?

lib/libc/regex/regcomp.c

65 ↗

(On Diff #29134)

I think it's worth a comment to clarify this struct.

[kevans]

In D10920#237723, @emaste wrote:

Presumably the tests on the base system tools all passed. Do you think it's worth requesting an exp-run?

Indeed, they do. Honestly, I don't think it's worth it; breakage resulting from getting the top-level parser wrong is catastrophic and really really obvious from the base system tests in my experience in working with it given the fundamental nature of the beast.

I'll add said comment shortly.

[kevans]

Comment on the usage of branchc

[emaste]

Approved.