Address the review on OCCT#1417 and re-cut carried patch 0018 to match

[gsdali]

OCCT#1417 is our upstream filing of carried
patch 0018 (#555, GCPnts degenerate count and duplicate end point). Maintainer gkv311
reviewed it on 2026-08-03
with three requested changes. It has been sitting unanswered since.

The three are not cosmetic in equal measure: the first is a design decision that lands differently
in our build than in a stock one, and it needs deciding rather than picking whichever wording is
shorter.

The three requests

1. The No_Exception duplicate guard, which is the real one

We wrote, in both GCPnts_QuasiUniformAbscissa::initialize and GCPnts_UniformAbscissa::initialize:

Standard_ConstructionError_Raise_if(theNbPoints <= 1, "...");
if (theNbPoints <= 1)
{
  // The check above is compiled out in a build defining No_Exception.
  myDone = false; myNbPoints = 0; return;
}

gkv311's objection is that *_Raise_if is an assert-grade macro, deliberately compiled out in
release for overhead, and that duplicating it is the wrong shape. He offers two fixes:

• (a) replace the macro with an unconditional if (...) throw Standard_ConstructionError(...);
• (b) "remove this duplicating check and keep only myDone=false;"

Read (b) carefully before implementing it. Taken literally in QuasiUniformAbscissa it is a
regression: deleting the block removes the return as well, and execution falls through into the
store to myParams, which is allocated empty for that count. That store is the entire defect #555
reported. What (b) has to mean is replace the _Raise_if with { myDone = false; myNbPoints = 0; return; }, not delete anything. Note that UniformAbscissa's site already assigns both fields
above the guard, so only the early return is load-bearing there, which is exactly the kind of
asymmetry that makes a one-line review instruction ambiguous.

Why the choice is ours to make and not a coin flip. We build with
BUILD_RELEASE_DISABLE_EXCEPTIONS=ON, which defines No_Exception, so today the macro is gone in
our kernel and the guard we added is the only thing standing there. Under (a) a degenerate count
starts throwing in our build, where it currently cannot; under (b) it stays a silent not-done.

Measure before choosing, do not reason it out:

• Which bridge functions reach these two initialize overloads, and does each sit inside a
  catch (...)? An uncaught Standard_ConstructionError crossing into Swift frames is a
  std::terminate(), which is the Uncatchable SIGABRT in parallel swift test run — cause unknown, very little evidence (see #344 for the companion SIGSEGV) #345 mechanism.
• What does the current Swift contract hand back for a degenerate count, and do GCPnts_UniformAbscissa/QuasiUniformAbscissa: two unpatched kernel defects behind #501's buffer-overflow fix (unbounded NbPoints(), degenerate-count SIGSEGV) #555's own tests
  in the repo assert it? Whichever option is chosen must not change that answer.
• Does anything in OCCT itself call these with a caller-supplied count it does not pre-validate?

If both options preserve our observable contract, prefer (b): it keeps the degenerate case a
not-done result rather than an exception, which is what #555 argued for, and it is the option that
does not depend on whether the consumer defines No_Exception. Say so in the reply, with the
measurement, rather than just complying.

2. Distance() in a loop

|| (aUU2 - aUi < aDelta && theC.Value(aUi).Distance(aPEnd) <= theTol3d)

OCCT convention is SquareDistance() against a tolerance squared once outside the loop. Mechanical,
but hoist the aTol2 computation properly rather than squaring inline each iteration.

3. theTol3d is misleading

The Perform template also instantiates on Adaptor2d_Curve2d, so the name asserts something false
for half its uses. Rename to theTol. Check for shadowing against the enclosing initialize's own
theTol at the call sites.

Scope

1. Update Scripts/patches/0018-GCPnts-degenerate-count-and-duplicate-end-point-555.patch to the
   revised form. This is the artifact of record; the upstream PR is downstream of it.
2. Re-verify against a rebuilt or override-linked kernel, not by reading the diff. GCPnts_UniformAbscissa/QuasiUniformAbscissa: two unpatched kernel defects behind #501's buffer-overflow fix (unbounded NbPoints(), degenerate-count SIGSEGV) #555's own
   note applies: override TUs for these files need -DNo_Exception to reproduce the build the
   patch is written for, and without it you will measure the wrong branch.
3. Re-run Scripts/repro/555-gcpnts-count-contract and confirm its verdict is unchanged. If the
   verdict moves, the review has changed behaviour and that is the finding, not a detail.
4. Run the GCPnts_UniformAbscissa/QuasiUniformAbscissa: two unpatched kernel defects behind #501's buffer-overflow fix (unbounded NbPoints(), degenerate-count SIGSEGV) #555 suites in the Swift tests. A degenerate count must still produce the same result it
   produces today.
5. Update Scripts/patches/README.md's 0018 section if the rationale changed.
6. Prepare the exact commit for gsdali/OCCT's fix/555-gcpnts-point-count branch as a patch file
   under the repro dir, with the reply to gkv311 drafted alongside it.

Do not push to gsdali/OCCT or comment on OCCT#1417. Both are outward-facing and need
authorisation. Prepare them and stop.

Not in scope

Changing what the fix does. The reviewer accepted the substance and asked about form; the two
degenerate-count contracts and the duplicate-end-point suppression stay as they are.

[github-actions]

This issue is missing a priority:* label (priority:P0–priority:P3). Please add the appropriate label so it shows up correctly in triage.

[gsdali]

Closed by PR #759.

All three review points addressed. The substantive one, the No_Exception guard, went to gkv311's option (b): Standard_ConstructionError_Raise_if replaced outright by a plain guard, rather than an unconditional throw added alongside it.

Taken literally in QuasiUniformAbscissa, option (b) would have been a regression, since deleting the block drops the return and execution falls through into the store to myParams that is the whole of #555. The implementation replaces the macro and keeps the return.

Measured before choosing, and the measurement went further than expected: no caller in the OCCT tree can reach the degenerate branch at all. Five production sites and one Draw command construct either class with a count, and every one hardcodes or clamps it first, uniformAbscissa's handler included with its own if (nocp < 2) return 1;. Neither header documents the exception either, so it was assert-grade rather than published contract. Both options were therefore free of consequence for OCCT; (b) was preferred because it does not depend on a build flag.

Patch 0018 changed, and it is in the pinned kernel. In a No_Exception build the old form already reduced to exactly the new one, so the only change that can move a compiled result is Distance() <= theTol becoming SquareDistance() <= aTol2, which differs only by rounding at the boundary; the 13,534-configuration sweep found 0 differences. A kernel rebuild is in flight regardless, to bring 0018's new form and 0025 into the pinned asset together.