Background
Public methods often accept raw arrays (T[], T[][]) for what are conceptually read-only inputs. In general C#, taking an array parameter doesn't by itself imply the callee will write into it. However, in this codebase there are real cases where the native interop layer does mutate the caller's array in place — via pinning (fixed) or [In, Out] P/Invoke marshaling. Because both the "pure input" and "mutated in place" patterns look identical at the call site (T[]), callers can't tell without reading the implementation whether an array argument is safe to reuse afterward or has been silently rewritten.
Concrete examples found
Cv2.Find4QuadCornerSubpix(InputArray img, Point2f[] corners, Size regionSize) (src/OpenCvSharp/Cv2/Cv2_objdetect.cs:214) — copies the native result back into the caller's corners array element-by-element. This mutation is undocumented in the XML doc comment.Cv2.StereoCalibrate(..., double[] distCoeffs1, double[] distCoeffs2, ...) (src/OpenCvSharp/Cv2/Cv2_calib.cs:641) — pins distCoeffs1/distCoeffs2 via fixed and native writes back through the pointer. Correctly documented as "Input/output vector of distortion coefficients" (src/OpenCvSharp/Cv2/Cv2_calib.cs:629).Cv2.CalibrateCamera(..., double[] distCoeffs, ...) (src/OpenCvSharp/Cv2/Cv2_calib.cs:311) — same mutation mechanism as above (fixed pointer, native writes back), but documented as "Output vector of distortion coefficients" (src/OpenCvSharp/Cv2/Cv2_calib.cs:302), omitting that it is also an input.
Discussion so far
While working on #2013 (PR #2035), we discussed whether public methods should prefer IEnumerable<T>/IReadOnlyList<T> over raw arrays for pure inputs, to avoid this ambiguity. Conclusions reached for that PR's scope:
- For 2D jagged inputs (
T[][]) pinned via ArrayAddress2<T> for a native call's duration, keeping T[][] is preferable (avoids an allocation from converting IEnumerable<IEnumerable<T>>, and matches existing convention).
- For 1D inputs,
IEnumerable<T> removes the ambiguity for genuinely non-mutating cases — but when a 1D array parameter sits next to a 2D array parameter in the same signature (e.g. RefineDetectedMarkers(Point2f[][] detectedCorners, int[] detectedIds, Point2f[][] rejectedCorners, ...)), keeping it as T[] for visual/API consistency was judged preferable to introducing a lone IEnumerable<T> in the middle.
Open questions
- Should we adopt (and document, e.g. in
.github/copilot-instructions.md) an explicit convention distinguishing "pure input" arrays from "input that is mutated via native pinning/[In, Out] marshaling"?
- Should mutating-array parameters be documented more consistently (fixing the
CalibrateCamera/Find4QuadCornerSubpix doc gaps above), and should the codebase be audited for more undocumented instances?
- Is there a lightweight convention (consistent "Input/output" doc wording, explicit
[In, Out] attributes rather than relying on default marshaling, etc.) that makes mutation visible to callers without introducing a wrapper type?
This is a design/documentation question, not a specific bug — no urgency, but worth having a place to converge on a convention.
Background
Public methods often accept raw arrays (
T[],T[][]) for what are conceptually read-only inputs. In general C#, taking an array parameter doesn't by itself imply the callee will write into it. However, in this codebase there are real cases where the native interop layer does mutate the caller's array in place — via pinning (fixed) or[In, Out]P/Invoke marshaling. Because both the "pure input" and "mutated in place" patterns look identical at the call site (T[]), callers can't tell without reading the implementation whether an array argument is safe to reuse afterward or has been silently rewritten.Concrete examples found
Cv2.Find4QuadCornerSubpix(InputArray img, Point2f[] corners, Size regionSize)(src/OpenCvSharp/Cv2/Cv2_objdetect.cs:214) — copies the native result back into the caller'scornersarray element-by-element. This mutation is undocumented in the XML doc comment.Cv2.StereoCalibrate(..., double[] distCoeffs1, double[] distCoeffs2, ...)(src/OpenCvSharp/Cv2/Cv2_calib.cs:641) — pinsdistCoeffs1/distCoeffs2viafixedand native writes back through the pointer. Correctly documented as "Input/output vector of distortion coefficients" (src/OpenCvSharp/Cv2/Cv2_calib.cs:629).Cv2.CalibrateCamera(..., double[] distCoeffs, ...)(src/OpenCvSharp/Cv2/Cv2_calib.cs:311) — same mutation mechanism as above (fixedpointer, native writes back), but documented as "Output vector of distortion coefficients" (src/OpenCvSharp/Cv2/Cv2_calib.cs:302), omitting that it is also an input.Discussion so far
While working on #2013 (PR #2035), we discussed whether public methods should prefer
IEnumerable<T>/IReadOnlyList<T>over raw arrays for pure inputs, to avoid this ambiguity. Conclusions reached for that PR's scope:T[][]) pinned viaArrayAddress2<T>for a native call's duration, keepingT[][]is preferable (avoids an allocation from convertingIEnumerable<IEnumerable<T>>, and matches existing convention).IEnumerable<T>removes the ambiguity for genuinely non-mutating cases — but when a 1D array parameter sits next to a 2D array parameter in the same signature (e.g.RefineDetectedMarkers(Point2f[][] detectedCorners, int[] detectedIds, Point2f[][] rejectedCorners, ...)), keeping it asT[]for visual/API consistency was judged preferable to introducing a loneIEnumerable<T>in the middle.Open questions
.github/copilot-instructions.md) an explicit convention distinguishing "pure input" arrays from "input that is mutated via native pinning/[In, Out]marshaling"?CalibrateCamera/Find4QuadCornerSubpixdoc gaps above), and should the codebase be audited for more undocumented instances?[In, Out]attributes rather than relying on default marshaling, etc.) that makes mutation visible to callers without introducing a wrapper type?This is a design/documentation question, not a specific bug — no urgency, but worth having a place to converge on a convention.