alamb commented on issue #146: URL: https://github.com/apache/arrow-rs/issues/146#issuecomment-5415976683
I took a look at the current state of `arrow-array/src/builder/` (as of
900ec3ee) to see if this issue is complete. Several unsafe callsites *are* now
documented (the `pub unsafe fn`s all have `# Safety` docs, and there are inline
`SAFETY` comments in `generic_list_builder.rs`,
`generic_bytes_view_builder.rs`, `generic_byte_run_builder.rs`, and others).
However, there are still **22 unsafe callsites in non-test builder code with
no safety comment** — almost all bare `unsafe { builder.build_unchecked() }`
calls. So I don't think this issue is complete yet. Here is the list:
| Method | Callsite |
|---|---|
| `BooleanBuilder::finish` |
[boolean_builder.rs#L170](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/boolean_builder.rs#L170)
|
| `BooleanBuilder::finish_cloned` |
[boolean_builder.rs#L184](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/boolean_builder.rs#L184)
|
| `FixedSizeBinaryBuilder::finish` |
[fixed_size_binary_builder.rs#L138](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/fixed_size_binary_builder.rs#L138)
|
| `FixedSizeBinaryBuilder::finish_cloned` |
[fixed_size_binary_builder.rs#L150](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/fixed_size_binary_builder.rs#L150)
|
| `FixedSizeBinaryDictionaryBuilder::finish` |
[fixed_size_binary_dictionary_builder.rs#L319](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/fixed_size_binary_dictionary_builder.rs#L319)
|
| `FixedSizeBinaryDictionaryBuilder::finish_cloned` |
[fixed_size_binary_dictionary_builder.rs#L338](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/fixed_size_binary_dictionary_builder.rs#L338)
|
| `FixedSizeBinaryDictionaryBuilder::finish_preserve_values` |
[fixed_size_binary_dictionary_builder.rs#L373](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/fixed_size_binary_dictionary_builder.rs#L373)
|
| `GenericByteBuilder::finish` |
[generic_bytes_builder.rs#L218](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/generic_bytes_builder.rs#L218)
|
| `GenericByteBuilder::finish_cloned` |
[generic_bytes_builder.rs#L233](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/generic_bytes_builder.rs#L233)
|
| `GenericByteDictionaryBuilder::finish` |
[generic_bytes_dictionary_builder.rs#L508](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/generic_bytes_dictionary_builder.rs#L508)
|
| `GenericByteDictionaryBuilder::finish_cloned` |
[generic_bytes_dictionary_builder.rs#L524](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/generic_bytes_dictionary_builder.rs#L524)
|
| `GenericByteDictionaryBuilder::finish_preserve_values` |
[generic_bytes_dictionary_builder.rs#L556](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/generic_bytes_dictionary_builder.rs#L556)
|
| `MapBuilder::finish_helper` |
[map_builder.rs#L289](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/map_builder.rs#L289)
|
| `NullBuilder::finish` |
[null_builder.rs#L91](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/null_builder.rs#L91)
|
| `NullBuilder::finish_cloned` |
[null_builder.rs#L100](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/null_builder.rs#L100)
|
| `PrimitiveBuilder::finish` |
[primitive_builder.rs#L331](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/primitive_builder.rs#L331)
|
| `PrimitiveBuilder::finish_cloned` |
[primitive_builder.rs#L345](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/primitive_builder.rs#L345)
|
| `PrimitiveDictionaryBuilder::finish` |
[primitive_dictionary_builder.rs#L448](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/primitive_dictionary_builder.rs#L448)
|
| `PrimitiveDictionaryBuilder::finish_cloned` |
[primitive_dictionary_builder.rs#L464](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/primitive_dictionary_builder.rs#L464)
|
| `PrimitiveDictionaryBuilder::finish_preserve_values` |
[primitive_dictionary_builder.rs#L496](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/primitive_dictionary_builder.rs#L496)
|
| `UnionBuilder::build` |
[union_builder.rs#L300](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/union_builder.rs#L300)
|
| `UnionBuilder::build_cloned` |
[union_builder.rs#L337](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/union_builder.rs#L337)
|
A few borderline cases I did *not* count (safety is arguably established,
but there is no comment at the callsite itself): the inner `get_unchecked`
calls in [`append_view_unchecked`
(generic_bytes_view_builder.rs#L214-L217)](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/generic_bytes_view_builder.rs#L214-L217)
(covered by the fn's `# Safety` doc) and the `append_view_unchecked` call in
[`try_append_view`
(generic_bytes_view_builder.rs#L279)](https://github.com/apache/arrow-rs/blob/900ec3ee38276ab651e210c4a85b38f8a8a61bcf/arrow-array/src/builder/generic_bytes_view_builder.rs#L279)
(preconditions checked immediately above). I also excluded 3 sites in
`#[cfg(test)]` code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
