docs: warn against using &v[0] on empty vectors in MakeSpan#2074
docs: warn against using &v[0] on empty vectors in MakeSpan#2074Madhav1729 wants to merge 2 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| // return absl::MakeSpan(&array[0], num_elements_); | ||
| // } | ||
| // | ||
| // NOTE: Do not use `&v[0]` or `&v[v.size()]` to construct spans from an empty |
There was a problem hiding this comment.
Thanks for submitting this PR and pointing out this common undefined behavior.
To keep the headers clean and concise, we prefer to avoid large code blocks in comments when the usage is already covered by existing examples. Also, if we had to educate users about all the possible undefined behavior in C++ everywhere it might be a problem, our documentation would be overwhelming.
How do you feel about condensing the warning to a more concise version? Something like:
// NOTE: To avoid undefined behavior if the container is empty, use `.data()`
// or pass the container directly instead of using `&v[0]` or `&v[v.size()]`.
This still warns developers about the UB risk with &v[0] / &v[v.size()] and guides them to the safe alternatives without taking up too much vertical space in the header.
There was a problem hiding this comment.
Sounds good to me. My main goal was to call attention to the UB risk with &v[0] / &v[v.size()], and I agree the shorter version communicates that without adding too much detail to the header.
I'll update the comment accordingly. Thanks for the feedback!
This PR adds a warning note to the documentation of absl::MakeSpan and absl::MakeConstSpan in absl/types/span.h regarding a common undefined
behavior (UB) pitfall when constructing spans from std::vector (or other dynamic containers).
The Problem
Developers frequently try to initialize spans using array-like pointer boundaries:
However, if v is empty, dereferencing v[0] to obtain its address is undefined behavior in C++. Similarly, attempting to use &v[v.size()] to obtain a range-end pointer also triggers UB.
The Fix
This documentation update advises developers to use .data() instead:
Or simply pass the container directly to the container overload of MakeSpan :
This addresses the common pitfall discussed in Issue #1334.
Related Issues:
Closes #1334