ICU-23509 Update the UnicodeSet parser to match the grammar that has the right precedence (no functional change)

[eggrobin]

The existing parser did not actually do what UTS61 said, so going back to a grammar that has the desired precedence (equal between union, intersection, and difference, instead of difference and intersection having higher precedence) has no effect.

How did the parser fail to implement the bad grammar?

See

icu/icu4c/source/common/uniset_props.cpp

Lines 1181 to 1192 in 2ebc705

	void UnicodeSet::parseRestriction(Lexer &lexer,
	UnicodeString &rebuiltPat,
	uint32_t options,
	UnicodeSet &(UnicodeSet::*caseClosure)(int32_t attribute),
	int32_t depth,
	UErrorCode &ec) {
	// Parse a https://www.unicode.org/reports/tr61/#Restriction:
	// Restriction ::= UnicodeSet
	// | Intersection
	// | Difference
	// Intersection ::= Restriction & UnicodeSet
	// Difference ::= Restriction - UnicodeSet

Instead of

icu/icu4c/source/common/uniset_props.cpp

Lines 1201 to 1203 in 2ebc705

	UnicodeSet leftHandSide;
	leftHandSide.parseUnicodeSet(lexer, rebuiltPat, options, caseClosure, depth + 1, ec);
	addAll(leftHandSide);

and then, in a loop,

icu/icu4c/source/common/uniset_props.cpp

Lines 1212 to 1215 in 2ebc705

	UnicodeSet rightHandSide;
	rightHandSide.parseUnicodeSet(lexer, rebuiltPat, options, caseClosure, depth + 1, ec);
	U_UNICODESET_RETURN_IF_ERROR(ec);
	retainAll(rightHandSide);

or

icu/icu4c/source/common/uniset_props.cpp

Lines 1229 to 1232 in 2ebc705

	UnicodeSet rightHandSide;
	rightHandSide.parseUnicodeSet(lexer, rebuiltPat, options, caseClosure, depth + 1, ec);
	U_UNICODESET_RETURN_IF_ERROR(ec);
	removeAll(rightHandSide);

a parser actually implementing what the grammar said would done leftHandSide.retainAll/removeAll, and leftHandSide.addAll only at the end of the Restriction.

Checklist

• Required: Issue filed: ICU-23509
• Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
• Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
• Issue accepted (done by Technical Committee after discussion)
• Tests included, if applicable
• API docs and/or User Guide docs changed or added, if applicable
• Approver: Feel free to merge on my behalf

[eggrobin]

@markusicu I have no idea what ticket to use here (ICU-23297 would fit nicely but went into 78, all the others listed in #4030 seem irrelevant to the basic grammar).

[eggrobin]

I guess the ticket question will also arise for the UnicodeSet documentation PR, and this fits the same theme (this is just internal documentation, I am changing comments and renaming internal identifiers).

[markusicu]

once you have a ticket, i could excavate my rubber stamp

[eggrobin]

I filed ICU-23509.

[eggrobin]

So... we follow the alternate grammar which is easier to parse

I think in order to make this easier to follow, I should make the parseMeow functions where Meow actually is represents an expression static, returning a set. (If I had done that, my parser from last year would have correctly parsed the grammar that gave - and & higher precedence, this would probably have broken everything, and I would have noticed the bug in the grammar a year earlier.)

Then the weird functions that parse a non-expression can remain mutating member functions, and the distinction will be much more obvious. This might also make the code more concise, as a lot of the usages of expression-parsing functions are UnicodeSet blah; blah.parseMeow().

But I will do that in a separate PR, so that this one only changes comments and renames stuff but preserves the logic.

[eggrobin]

(This will get a couple more changes, Roozbeh spotted a mistake in the grammar. The comments will actually get simpler, for a change.)

[eggrobin]

Should match the grammar in https://github.com/unicode-org/unicode-reports/pull/330 now.

[markusicu]

... I should make the parseMeow functions where Meow actually is represents an expression static, returning a set. ...

But I will do that in a separate PR, so that this one only changes comments and renames stuff but preserves the logic.

sg -- please create a ticket

[markusicu]

rubber stamp

[markusicu]

optional: please avoid dot space space...

[markusicu]

oh, spotless wants to wrap another line -- whose overflow then wants to be merged into the following stuff

[jira-pull-request-webhook]

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

[eggrobin]

sg -- please create a ticket

I might reuse ICU-23257.

[markusicu]

sg -- please create a ticket

I might reuse ICU-23257.

sgtm

[markusicu]

all checks have passed, ready to merge (approver not encouraged to do so)