Repository navigation
Fix #360: validate tensor shapes at construction - #438
Merged
Merged
Conversation
Shape errors used to build silently and surface as out-of-bounds reads in the evaluator. Tensor +/-/* , inner_product, otimes/otimesu/otimesl, dot_product and dot now throw invalid_expression_error (not assert) in the factories, before the zero short-circuits, and in the node constructors so rebuild/substitution paths are covered too. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
…-time rejection substitute(sym(A), A, E) with mismatched dims now throws invalid_expression_error while rebuilding the contraction, so the tag/evaluation assertions it made are unreachable. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
This was referenced Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #360.
Shape errors used to construct silently and surface later as out-of-bounds reads or garbage sums in the evaluator. All checks now throw
invalid_expression_error(never assert), so they fire in Release too.New shared header
tensor/tensor_shape_validation.h; checks run in the factories before the zero short-circuits and in the node constructors, so rebuild/substitution paths are covered as well.+/-tensor_zeroof another shape, and n-ary chains like(X+Y)+C)*inner_product(+inner_product_wrapperctor)otimes/otimesu/otimesl(+outer_product_wrapperctor)dot_product(+tensor_inner_product_to_scalarctor)assert(... || ...)→ thrown full-contraction check: every index of both operands consumed (a scalar result requires&&), dims equal, indices validdot(A)dcontract_self_op) is rank-2 onlyChanged existing test (separate commit)
RoundTwoReview.DimChangingSubstitutionDropsTag→DimChangingSubstitutionThrows(tests/CoreBugFixTest.h).substitute(sym(A), A, E)with A dim 3 and E dim 2 was expected to succeed, produce an untagged result (EXPECT_FALSE(is_symmetric(s))), and throwevaluation_erroronly when evaluated.invalid_expression_errorat construction. The tag and evaluation assertions are unreachable and were dropped; the test was renamed to match what it asserts.Kept in its own commit so it can be reviewed or reverted independently of the fix.
Tests
6 new
TensorShapeValidation.*tests intests/TensorExpressionTest.h, covering each rejection above plus zero-operand short-circuits, directmake_expressionconstruction, and cases that must still be accepted (X+Y,C:{3,4}X:{1,2},otimesu(X,X),dot_product(X,{1,2},Y,{2,1}),dot(X)).Negative control: against unfixed code every rejection assertion fails (no crash); with the fix all pass.
Suite: Debug (gcc-14,
-Werrorminus the pre-existing pedantic/deprecated-declarations warnings) 2352/2352; Release 2352/2352 — the point of the issue is that validation is not assert-only. Fuzz binary unchanged (13 known rank-limit skips). clang-format-18 clean.Review note — coverage deliberately dropped. The replaced test also pinned a round-3 finding ("evaluation must throw rather than over-read the buffer"). That assertion is unreachable now: the malformed node cannot be built by any path, since both the factory and the
inner_product_wrapperconstructor validate (verified). Recorded here so the lost assertion is a decision, not an omission.Parser interaction: shape errors now escape
parse()ascas_errorwithout position info (e.g.parse("A + W"),parse("dot(u)")). Noted on #282, which tracks position threading for inside-dispatch errors.