test(codegen): the sweep sees a divisor scaled from the guarded value (#390) - #392
Merged
Merged
Conversation
…TA-Lib#390) TA-Lib#390 reports STOCH/STOCHF returning TA_SUCCESS with inf/nan for well-formed OHLC: stoch.c:230 guards `highest - lowest` and :231 divides by `diff`, which :189 sets to `(highest - lowest)/100.0`. A denormal range leaves the guarded expression non-zero while the divisor underflows to exactly 0.0. divisor_guard_suite did not see it, and could not: its one shape is a divisor accumulated across loop iterations, and `diff` is a fresh assignment. So the sweep read clean on a defect of exactly the class it exists for. Adds a second shape. A divisor assigned from an expression that MULTIPLIES OR DIVIDES is recorded with the variables that expression reads; if no guard tests the divisor but some guard tests one of those reads, that is reported. Addition and subtraction are excluded -- they cannot turn a guarded-non-zero into a zero divisor the way a scaling can. It flags STOCH and STOCHF, and nothing else in 202 functions. Both are annotated OPEN rather than patched: moving the guard onto `diff` changes their output on those windows, which is a decision about the functions. The new arm is pinned in the direction the existing self-tests do not cover. ER and VORTEX prove the sweep goes loud on a reintroduced defect; this one moves STOCH's guard onto `diff` and requires the finding to disappear, so a check that flagged every scaled divisor unconditionally would fail it.
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.
#390 found a defect that
divisor_guard_suiteshould have caught and did not. Thiscloses that gap in the sweep; it does not change STOCH or STOCHF.
Why the sweep was blind to it
The suite had one shape: a divisor accumulated across loop iterations with no guard
testing it. STOCH's
diffis a fresh assignment, so it never entered the accumulatorset and the sweep read clean — on a defect of exactly the class the suite exists for.
The shape it missed is a different one, and #390 states it exactly: the guard tests one
expression and the code divides by another.
stoch.c:230guardshighest - lowest,:231divides bydiff, and:189setsdiff = (highest - lowest)/100.0. A denormalrange leaves the guarded expression non-zero while the divisor underflows to exactly
0.0, so the guard proves something true about a number that is not the divisor.
The second arm
A divisor assigned inside a loop from an expression that multiplies or divides is
recorded together with the variables that expression reads. If no guard tests the
divisor, but some dominating guard tests one of those reads, that is a finding.
Addition and subtraction are deliberately excluded: they cannot send a
guarded-non-zero to a zero divisor the way a scaling can, and including them would bury
the real rows.
Over 202 input functions it flags STOCH and STOCHF, and nothing else — the two
#390 names.
Pinned in the direction the existing self-tests do not cover
The ER and VORTEX self-tests prove the sweep goes loud on a reintroduced defect. A
check that flagged every scaled divisor unconditionally would pass both of those and
still be worthless, so this arm is pinned the other way:
the_scaled_arm_clears_when_the_guard_moves_to_the_divisorrewrites STOCH's guard ontodiff— the fix #390 suggests — and requires the finding to disappear. It fails if thecheck is not actually reading the guard.
STOCH/STOCHF are annotated OPEN, not patched
Moving the guard onto
diffchanges STOCH's output on those windows, so it is adecision about a shipped function rather than something to slip in under a sweep — the
same line drawn for KAMA in #383 and settled by ruling in #385. #390 also notes the
frozen pre-cutover reference shares the defect, so the corrected values need Tulip or
pandas-ta as an oracle; that is not something a test suite can decide.
no_annotation_is_stalealready covers the follow-through: whenever the guard doesmove, these two rows stop being flagged and the suite says so rather than leaving a row
asserting a closed question.
Not covered, stated rather than implied
#390's second half — KAMA's quotient overflowing when
sumROC1is tiny-but-non-zeroand
periodROCis large — is not detected by this change. It is not a missingdenominator test at all: #385 added the exact zero guard and it holds. The defect is an
unbounded quotient, which needs a check about the result rather than about the divisor,
and I did not want to claim coverage the suite does not have.
Verification
cargo clippy --all-targetsclean, 936 generator tests green,ta_regtestgreen. Onefile changed; no generated source touched.