deprecate pyo3_ffi::c_str! - #6278
Conversation
|
It is still quite useful as it accepts expressions, so I don't think we should get rid of it entirely. I wonder how much it is used like that outside pyo3. |
A search on GitHub doesn't seem to return any repositories other than Pyo3 forks. |
|
There's a bunch more usages of |
Merging this PR will degrade performance by 10.24%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | bench_fast |
6.1 µs | 6.8 µs | -10.24% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing chirizxc:pyo3_ffi_c_str! (03f02f6) with main (92e47e1)
LilyFirefly
left a comment
LilyFirefly
left a comment
There was a problem hiding this comment.
On reflection I also think we shouldn't deprecate c_str! for expressions - the old version is more readable in those cases. I do like the clean-up available when we can use C string literals (c"...").
| let py_foo = c_str!(include_str!(concat!( | ||
| let py_foo = CStr::from_bytes_with_nul(concat!( | ||
| env!("CARGO_MANIFEST_DIR"), | ||
| "/python_app/utils/foo.py" | ||
| ))); | ||
| let py_app = c_str!(include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/python_app/app.py"))); | ||
| "/python_app/utils/foo.py", | ||
| "\0" | ||
| ).as_bytes()).unwrap(); | ||
| let py_app = CStr::from_bytes_with_nul(concat!( | ||
| env!("CARGO_MANIFEST_DIR"), | ||
| "/python_app/app.py", | ||
| "\0" | ||
| ).as_bytes()).unwrap(); |
There was a problem hiding this comment.
I don't think this is an improvement, so like @mejrs I think keeping cstr! for expressions would be better.
3 participants
No description provided.