Repository navigation
Print the brackets an operator that is not associative has - #1009
Conversation
Stringize is the library's own input format, so parsing what it prints has to
give back the expression it printed. Five operators broke that in one way: the
printer left the *right* operand unbracketed at its own precedence level, while
the grammar folds that level to the left, so the text came back re-associated.
Four of the five are not associative, so the answer moved and not merely the
shape. Measured on a build of 2.3.0 and a build of this branch:
false implies (true implies false) printed False implies True implies False
read back as (False implies True) implies False: True -> False
{ 1, 2, 3 } \ ({ 2, 3 } \ { 3 }) printed without its brackets:
{ 1, 3 } -> { 1 }
{ 1, 2 } \/ ({ 3 } \ { 1, 2 }) \/ and \ share one precedence level:
{ 1, 2, 3 } -> { 3 }
2 * (3 mod 2) mod shares a level with * and /:
2 -> 0
implies had the rule the wrong way round rather than missing: it bracketed the
assumption, which a left fold never needs, and not the conclusion. So
(a implies b) implies c loses its redundant brackets here, which is what made
the genuinely ambiguous case look no different from it.
\/ and * are bracketed only against the operator that makes them ambiguous,
since both are associative themselves and share a level with one that is not:
A \/ (B \/ C) still prints flat, A \/ (B \ C) does not; x * (y * z) and
x * (y / z) still print flat, x * (y mod z) does not.
What does not change, deliberately: an operator that *is* associative still
prints flat. x + (y + z) prints x + y + z and comes back as (x + y) + z -- a
differently shaped tree and the same number, because the bracketing carries no
mathematics. Bracketing those would print every expanded polynomial as a
right-nested pile of parentheses. StringizeRoundTripTest pins both halves so
the flat half stays a decision rather than an oversight.
Latexize moves for the set and mod cases and not for implies.
CSharpMath.Evaluation, which reads our LaTeX back, folds \cup, \setminus, \in,
\cdot and \bmod to the left at the same precedences this grammar uses, and
folds \to to the right -- the usual convention for implication, and what
Latexize was already bracketing for. The change only adds \left( \right)
groups, which CSharpMath already parses, so nothing is owed downstream (#822).
Syntax.md said everything but ^ groups to the left; provided groups to the
right, and has since it was written.
Suite 7591 passed, 1 failed, 14 skipped. The failure is
OneSidedLimitTest.ADifferenceOfReciprocalLogarithms, a Task.Wait timeout guard
that a different limit test tripped on the previous run and that passes when
run alone -- contention, not this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011emtnRT6EWTrxXNtqDVK3e
|
Implies associativity should be changed eventually to make it in line with how other people write it. Same for provided - left associativity is not useful because an AND operator within the condition represents the expression already. |
`provided` is the one infix operator the grammar folds to the right, so it is the *left* operand that has to say when it is an attached condition of its own. `(x provided p) provided q` printed as `x provided p provided q` and read back as `x provided (p provided q)`. It was classified as safe here on the strength of a value test, and the value is genuinely safe: both readings are `x` exactly when `p` and `q` hold. The contract is about the expression, not the number, and asking the expression is what catches it -- so the case moves from the flat-and-keeps-its-value theory to the keeps-its-grouping one. LatexTests.Provided3 and Provided4 asserted the flat form. They were pinning the ambiguity rather than a behaviour: both build a left-nested Providedf and expect it to print without the grouping that distinguishes it. Updated, with the reason beside them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011emtnRT6EWTrxXNtqDVK3e
|
Thank you — and the second half of that comment found a defect in this PR, so I have pushed a fix rather than only replying.
provided_expression returns[Entity value]
: expr = implies_expression { $value = $expr.value; }
('provided' pred = provided_expression { $value = $value.Provided($pred.value); })?
// note: even though Provided is associative, we parse it right-to-left matching natural language
// "I'll go, provided you go, provided it's sunny" - Natural reading: I'll go ← (you go ← sunny)What was wrong was But that made Because it folds right, it is the left operand that mis-associates — the mirror of every other case in this PR. The round-trip contract is about the expression, and asking the expression is what catches it. Fixed in On
|
FromStringTest.TestProvided4 asserted that `a provided (b provided c)` and `(a provided b) provided c` produce the same string. They are different expressions -- the parse is right-associative, which the two tests above it pin -- so that assertion was the round trip failing, written down as an expectation. It now asserts what the contract asks: the two print differently, and each reads back as itself. Found by CI rather than locally, because the run before this one was filtered to the printer tests and this one lives with the parser. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011emtnRT6EWTrxXNtqDVK3e
|
Maybe an issue on pending breakages would be appropriate. Remember to note to yourself to follow through on the next major version. Also, maybe noting down to promote experimentals and removing obsoletions would be appropriate. |
|
Done — #1019, "Pending breakages: changes agreed or arguable that cannot ship before a major version." It opens with the criterion, since that is the part worth being strict about: a change belongs there when it moves the value of existing input or removes a published member. A changed answer has gone in a minor here, but every one of those was a wrong answer becoming right, and a deliberate convention change should not borrow that licence. On it now:
And the follow-through note is in the issue itself rather than in my head: it says it is to be read before the next major version is named, the same way #746 is read before any version is named. This PR is unaffected — it makes the printer state the grouping the grammar actually has, which is right whichever way the grammar later folds, and the round-trip tests here keep the flip honest when it happens. |
Master gained the fix while this branch was open. a implies (b implies c), (a provided b) provided c and the set-minus case now survive the round trip, so the test asserts that instead of asserting the defect, and the two documents say associative rather than naming implies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd
* Serialize an Entity as the expression it prints (#323) JsonSerializer.Serialize(entity) threw for every entity, (Entity)3 included: Nodes is a node's enumeration of itself, so the reflecting object converter walked it until it reported an object cycle. DataContractSerializer refused the types outright and Entity was never [Serializable]. So an expression could not be a member of any serializable type. The format is the printed form. It is an exact serialization already, and one the library has to keep exact whatever else it does, because it is also the input format -- so this inherits that contract instead of adding a second description of the same tree with its own per-node code and its own way of drifting. Reading costs about 430 us and 0.8 MB for a 43-node expression against 6.5 us and 25 kB to build the tree from constructors, which is an argument for a faster parser rather than for a second representation. The attribute is on every public node type and not only on Entity, because System.Text.Json looks it up with inherit: false and a member declared as Entity.Variable would otherwise reach the object converter and the cycle. EntitySerializationTest names any node type that is missing, so a new one cannot join the list silently, and step 11 of AddingNode.cs says so. EntityJsonConverterAttribute overrides CreateConverter so the converter is reached statically rather than through Activator.CreateInstance; the trim and AOT analyzers report nothing on this code. Compiled for net8.0 and later, since netstandard2.0 has no System.Text.Json in the box and the csproj takes no package reference for it. Function and TrigonometricFunction had to become partial to be named. What the printed form does not carry, measured over every node type: a Codomain, which nothing prints (#1022, filed, and pinned here by a test written to fail when it is fixed), and a Complex with both parts non-zero, which prints as a sum -- already recorded in EveryNodeSurvivesEveryPipelineTest. Everything else round trips, binders and bound constants included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd * Record what a right nesting does to the round trip (#1032) Measuring the claim rather than repeating it: an operator whose operands nest to the right comes back nested to the left, because the printed form does not bracket a right operand of equal priority. For +, *, and, or and xor that is shape and not value, since each is associative -- which is the whole of what is left of #323's objection that a.ToString().ToEntity() need not equal a. For implies it is value. a implies (b implies c) prints as a implies b implies c, which reads back as (a implies b) implies c, and the two disagree at a=False, b=True, c=False. Filed as #1032, in the printer's parenthesisation guard, with Providedf the mirror of it. Pinned here in both directions so the entry cannot outlive it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd * Assert the brackets that #1009 put back, rather than their absence Master gained the fix while this branch was open. a implies (b implies c), (a provided b) provided c and the set-minus case now survive the round trip, so the test asserts that instead of asserting the defect, and the two documents say associative rather than naming implies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t say (#746 items 8, 74) (#1041) Syntax.md is the only statement of the language other than the grammar itself, so what it leaves out is undiscoverable and what it gets wrong is worse than nothing. Derived rather than taken from a list: every function-open literal in AngouriMath.g -- 102 of them -- checked against the page, plus the non-function productions of `atom`, the lexer rules, and the token insertion in Core/Parser.cs. Twenty function names and fifteen features were missing. Added: sum and product, the operators that declare a name over a range, with what the declared name shadows: `sum(i, i, 1, 10)` is 55 while `sum(sqrt(-1) * i, i, 1, 10)` is 55i, and `product(pi, pi, 1, 4)` is 24 exp, log10, log2, and log with one argument, which is base 10 the a- spellings of every inverse hyperbolic and the five refusals beyond arcsinh (|x|), the absolute-value brackets True and False, which is what Stringize prints // and /* */ comments, and newlines, which are skipped .5, 1., and the imaginary suffix in 3i and 1.5e3i MathS.Settings.ExplicitParsingOnly Cyrillic letters in names, where only Greek was named Corrected, each with the probe that settles it: The variable-name rule said "a letter or `_` followed by letters, digits or `_`". The grammar is `letter+ ('_' (letter|digit)+)?`, which disagrees four ways: `_x` and `x_` are lexer errors, `x_1_2` is one too, `x1` is `x ^ 1` rather than a name, and Cyrillic is a letter as much as Greek. "`sinx` is `s * i * n * x`" is false -- `sinx` is one variable named `sinx`, because the lexer takes the longest match. Nothing here produced a product of one-letter names. "Juxtaposition is multiplication" is half of it. A number, a name or `)` followed by a number inserts `^` and not `*`, so `x2` is `x ^ 2` while `x(2)` is `x * 2`, and `x i` is `x ^ i` because `i` is a number token. The associativity statement item 74 also names -- everything but `^` groups to the left -- was already corrected by #1009 and is left as it stands. Two things the page now records rather than fixes: Stringize drops a codomain (#1022), and a // comment that ends the input is a parse error because the lexer rule requires the newline (#1039, filed from this work). #1028 gained the note that `[]` reaches its IndexOutOfRangeException through the parser and not only through MathS.Vector. Sources/Tests/UnitTests/Convenience/SyntaxDocumentedTest.cs runs every example on the page: the precedence table row by row, the four shapes that are not names, both juxtaposition rules and ExplicitParsingOnly, the number and boolean spellings, comments, the inverse hyperbolic table, and sum and product including what their declared name shadows. It compares entities, never printed forms. Suite: 7930 passed, 0 failed, 14 skipped, against 7825 / 0 / 14 on this branch before the change -- the 105 are this file, and nothing else moved. The new file carries the default 2019-2022 header; Sources/.editorconfig, which pins it, belongs to other pull requests this cycle and no 2026 scope was added there. Claude-Session: https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Stringizeprints an expression that parses back to a different expression, and in one case to a different truth value. The printed form is supposed to be a lie-free channel —AGENTS.mdcalls it a contract andStringizeRoundTripTestis where it is enforced — so this is the silent kind of defect that contract exists to prevent: a wrong reading is still a valid expression, so nothing throws.Five of the six change the value, not only the tree:
false implies (true implies false)=TrueFalse{1,2,3} \ ({2,3} \ {3})={ 1, 3 }{ 1 }{1,2} \ ({2} \/ {3})={ 1 }{ 1, 3 }{1,2} \/ ({3} \ {1,2})={ 1, 2, 3 }{ 3 }2 * (3 mod 2)=20The rule, and why it is one rule
The grammar folds every binary operator to the left except
^andprovided. So the printed form of a left-nested tree needs no brackets and the printed form of a right-nested one does — exactly when the operator, together with the others sharing its precedence level, is not associative. That is the rule-,/,modand^already follow (<=on the right); the operators fixed here were the ones that did not.Two of these are not obvious from the operator alone, which is why they were missed:
\/is associative and still mis-associated, because\/and\share one precedence level and are folded by one loop in the grammar. Union-on-union is safe; a set difference on the right of a union is not.*is associative and still mis-associated, becausemodsits at the same level as*and/.implieswas a third case: the printer had<=on the left and<on the right — the rule the right way round for a right-associativeimplies, which is the conventional reading but not this grammar's. So it was over-bracketing(a implies b) implies cand under-bracketing the one that mattered, and the redundant brackets on the safe case are what made the ambiguous one look no different.Same-shape verdict on every binary operator
AGENTS.md: "ask what else is the same shape, and fix that too, or write down why not."implies,\,\/(against\),in,*(againstmod)+,and,or,xor,/\,\/(against\/),*(against*,/)provided-,/,mod,^,!, the comparisonspiecewise,lambda,apply, matrices, function callsBracketing the associative ones as well would be the strict entity-level reading of the contract, and it is deliberately not done here:
"(a + b + c + d)^2".Expand()is right-nested throughout and would print as a ten-deep pile of parentheses. Both halves are now pinned by tests, so the flat half is a recorded decision rather than an oversight.LaTeX
Fixed in the same shapes. I read CSharpMath.Evaluation's precedence table rather than assuming: it folds
\cup,\setminus,\in,\cdotand\bmodleft at the same groupings this grammar uses, so those get the same treatment — and it folds\toright, which is whatLatexize'simplieswas already bracketing for, so that one is unchanged and was already correct. The change only ever adds\left(/\right)groups, which CSharpMath parses, so no matching PR is owed downstream (#822).Measured
AMisreadGroupingWouldChangeTheValueevaluates both sides,ANonAssociativeOperatorKeepsItsGroupinground-trips the tree,AnAssociativeOperatorPrintsFlatAndKeepsItsValuepins the deliberate flat half.BREAKING-CHANGES.md, old value and new, measured on a build of each version rather than read off the diff.EveryNodeSurvivesEveryPipelineTestpasses andKnownRoundTripFailuresis untouched — that test builds one sample node per type with leaf children and never nests two operators at one precedence level, which is precisely the hole these defects lived in.Docs/Usage/Syntax.mdsaid "everything groups to the left except^". That was false forprovidedand had been since it was written. Corrected, with the two shared precedence levels noted.Two things worth a maintainer's eye
impliesassociativity is a language decision, not a printer one. Mathematics, Lean, Coq, Agda, Haskell and CSharpMath all read→as right-associative; this grammar folds it left andSyntax.mddocuments that as deliberate. This PR makes the printer match the documented grammar. Changing the grammar instead is defensible and much larger — it moves the value of existing user input.Stringizeis used as a dictionary key inCommonDenominatorSolver.cs:95. While the round trip was broken, structurally distinct denominators could collide on one key. This narrows it; keying on printed text is a latent hazard worth its own look.