Clarify wide column memory ownership in public APIs (#14626)#14626
Clarify wide column memory ownership in public APIs (#14626)#14626xingbowang wants to merge 1 commit into
Conversation
✅ clang-tidy: No findings on changed linesCompleted in 0.0s. |
✅ Claude Code ReviewAuto-triggered after CI passed — reviewing commit 7744d0e Code Review: Clarify wide column memory ownership in public APIsScope: Documentation-only change (6 files, +43/-12 lines) The core documentation claims are correct — F1 — Unsafe example comment formatting (MEDIUM)
// // Unsafe: the temporary std::string storage is destroyed before
// PutEntity(). WideColumns bad_columns{Suggest using consistent formatting, e.g.: // Unsafe example — the temporary std::string objects are destroyed before
// PutEntity() is called:
// WideColumns bad_columns{F2 — ASSERT_OK in public header example (LOW)
F3 — Missing documentation on additional PutEntity APIs (LOW)A few public APIs taking
Documentation content is technically correct and addresses a real usability problem. Findings are presentation/coverage suggestions, not correctness issues. ℹ️ About this responseGenerated by Claude Code. Limitations:
Commands:
|
|
@xingbowang has imported this pull request. If you are a Meta employee, you can view this in D101280295. |
Summary: We have seen multiple coding error with the usage of WideColumn API. Hence, improving the documentation to reduce the chance of this happening again. Clarify that `WideColumn` and `WideColumns` are non-owning views over caller-managed memory. Update the public `PutEntity()` API documentation to state that column name and value storage must remain valid until the call returns. Add safe and unsafe usage examples in `wide_columns.h` to make lifetime requirements explicit. ## Testing Not run. Documentation-only change. Reviewed By: archang19 Differential Revision: D101280295 Pulled By: xingbowang
7744d0e to
62efb98
Compare
|
@xingbowang has exported this pull request. If you are a Meta employee, you can view the originating Diff in D101280295. |
|
@xingbowang merged this pull request in c9252b9. |
Summary:
We have seen multiple coding error with the usage of WideColumn API. Hence, improving the documentation to reduce the chance of this happening again.
Clarify that
WideColumnandWideColumnsare non-owning views over caller-managed memory.Update the public
PutEntity()API documentation to state that column name and value storage must remain valid until the call returns.Add safe and unsafe usage examples in
wide_columns.hto make lifetime requirements explicit.Testing
Not run. Documentation-only change.
Reviewed By: archang19
Differential Revision: D101280295
Pulled By: xingbowang