We should fix this warning eventually. Silence it for now to make Clippy
pass without warnings, which makes it much more useful.
Compiling fish-rust v0.1.0 (/home/johannes/git/fish-riir/fish-rust)
error: mutable borrow from immutable input(s)
--> src/ffi.rs:79:32
|
79 | pub fn get_procs(&self) -> &mut [UniquePtr<process_t>] {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: immutable borrow here
--> src/ffi.rs:79:22
|
79 | pub fn get_procs(&self) -> &mut [UniquePtr<process_t>] {
| ^^^^^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mut_from_ref
= note: `#[deny(clippy::mut_from_ref)]` on by default
error: could not compile `fish-rust` due to previous error
A following commit will pass global string constants to the gettext macro.
This is not ideal because we might accidentally use the constants without
gettext (which we should never do). To fix that we might need to define a
macro per constant, or use a proc macro which is maybe not worth it.
warning: deref which would be done by auto-deref
--> src/wchar_ffi.rs:81:5
|
81 | &*EMPTY_WSTRING
| ^^^^^^^^^^^^^^^ help: try this: `&EMPTY_WSTRING`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref
= note: `#[warn(clippy::explicit_auto_deref)]` on by default
lazy_static has better ergonomics at the call/access sites (it returns a
reference to the type directly, whereas with once_cell we get a static Lazy<T>
that we must dereference instead) but the once_cell api is slated for
integration into the standard library [0] and has been the "preferred" way to
declare static global variables w/ deferred initialization. It's also less
opaque and easier to comprehend how it works, I guess?
(Both `once_cell` and `lazy_static` are already in our dependency tree, so this
should have no detrimental effect on build times. It actually negligibly
*improves* build times by not using macros, reducing the amount of expansion the
compiler has to do by a miniscule amount.)
[0]: https://github.com/rust-lang/rust/issues/74465
It's debatable whether is_ascii_digit() is better than (0..=9).contains().
(Probably we want to go with the mainstream Rust choice eventually.)
Let's disable the warning for now since it's not terribly important.
We should only be dealing with wcharz_t at the language boundary.
Rust callers should prefer the equivalent &wstr.
Since wcsfilecmp() is no longer exposed directly it can take &wstr only.
This is early work but I guess there's no harm in pushing it?
Some thoughts on the conventions:
Types that live only inside Rust follow Rust naming convention
("FeatureMetadata").
Types that live on both sides of the language boundary follow the existing
naming ("feature_flag_t").
The alternative is to define a type alias ("using feature_flag_t =
rust::FeatureFlag") but that doesn't seem to be supported in "[cxx::bridge]"
blocks. We could put it in a header ("future_feature_flags.h").
"feature_metadata_t" is a variant of "FeatureMetadata" that can cross
the language boundary. This has the advantage that we can avoid tainting
"FeatureMetadata" with "CxxString" and such. This is an experimental approach,
probably not what we should do in general.
The initial port of feature flags requires a global initialization. Since
fish_indent accesses feature flags, let's make sure to initialize them here.
In future, we can stop initializing things fish_indent doesn't need (like
the topic monitor) but that's no big deal. Global initialization should
always be a benign addition.
The original implementation without the test took me 3 hours (first time
seriously looking into this)
The functions take "wcharz_t" for smooth integration with existing C++ callers.
This is at the expense of Rust callers, which would prefer "&wstr". Would be
nice to declare a function parameter that accepts both but I don't think
that really works since "wcharz_t" drops the lifetime annotation.
rustfmt removes the "::" prefix from qualifiers. This breaks the build because
I think a later "pub use ffi::*" results in "std" being an ambiguous reference.
We can re-enable these once we're nearing a RIIR release (or if someone thinks
it's a good use of their time to fix them before then). Otherwise we're just
going to have GitHub reporting CI failure for all commits instead of just the
ones that actually broke something.
(I'm mainly trying to get the branch in a good state to merge into master.)
