crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.22k stars 1.61k forks source link

Rename `StaticArray#to_slice` to `#to_unsafe_slice` #14613

Open straight-shoota opened 1 month ago

straight-shoota commented 1 month ago

StaticArray#to_slice is unsafe because it returns an unprotected pointer to data on the stack. This pointer can easily be overriden which invalidates the slice (see #14610). Renaming to #to_unsafe_slice highlights the unsafe nature of using the method, and distinguishes it from other #to_slice implementations that are inherently more safe because they return heap pointers.

StaticArray#to_slice becomes deprecated.

The first commit replaces StaticArray where it is used in the spec suite as an example for the implementation of #to_slice. Using a String literal with the same payload seems like a good idea. It even simplifies those specs.

There are quite a lot of calls to StaticArray#to_slice in the standard library and spec suite. All of them get replaced. It's interesting to note that they're all explicit calls to StaticArray#to_slice and do not involve dynamic dispatch to some other #to_slice method. This makes rewriting easy and confirms that having StaticArray break from the unified #to_slice interface does not cause any practical problems. Calls to StaticArray#to_slice are typically unrelated to calls to other #to_slice methods.

Replaces part of #11031 and resolves part of https://github.com/crystal-lang/crystal/issues/9606

Depends on #14615

straight-shoota commented 1 month ago

@ysbaddaden Yeah it would be super useful to a semantically aware tool to automatically rewrite calls to deprecated methods 🤔

straight-shoota commented 1 month ago

Interpreter specs are broken due to #14615