This was added to support signals; however we are unlikely to use this
for anything else. Remove it; just use a u64 to report signals that have
been set.
This optimizes over both the rust rewrite and the original C++ code. The rust
rewrite saw `std::bitset` replaced with `[bool; 65]` which could result in a
lot of memory copy bandwidth each time we checked for and received no signals.
The original C++ code would iterate over all signal slots to see if any were
set. The code now returns a single u64 and only checks slots that are known to
have signals via an intelligent `Iterator` impl.
You can now use a reference to CxxWString or an allocated UniquePtr<CxxWString>
to get an &wstr temporary to use without having to allocate again (e.g. via
`from_ffi()`).
wchar.rs should not import let alone reexport FFI strings.
Stop re-exporting utf32str! because we use L! instead.
In wchar_ffi.rs, stop re-exporting cxx::CxxWString because that hasn't
seen adoption.
I think we should use re-exports only for aliases like "wstr" or for aliases
into internal modules.
So I'd probably remove `pub use wchar_ffi::wcharz_t = crate::ffi::wcharz_t`
as well.
bool_assert_comparison is stupid, the reason they give is "it's shorter". Well,
`assert!(!foo)` is nowhere near as readable as `assert_eq!(foo, false)` because
of the ! noise from the macro.
Uninlined format args is a stupid lint that Rust actually walked back when they
made it an official warning because you still have to use a mix of inlined and
un-inlined format args (the latter of which won't complain) since only idents
can be inlined.
This shows some of the ugliness of the rust borrow checker when it comes to
safely implementing any sort of recursive access and the need to be overly
explicit about which types are actually used across threads and which aren't.
We're forced to use an `Arc` for `ItemMaker` (née `item_maker_t`) because
there's no other way to make it clear that its lifetime will last longer than
the FdMonitor's. But once we've created an `Arc<T>` we can't call
`Arc::get_mut()` to get an `&mut T` once we've created even a single weak
reference to the Arc (because that weak ref could be upgraded to a strong ref at
any time). This means we need to finish configuring any non-atomic properties
(such as `ItemMaker::always_exit`) before we initialize the callback (which
needs an `Arc<ItemMaker>` to do its thing).
Because rust doesn't like self-referential types and because of the fact that we
now need to create both the `ItemMaker` and the `FdMonitorItem` separately
before we set the callback (at which point it becomes impossible to get a
mutable reference to the `ItemMaker`), `ItemMaker::item` is dropped from the
struct and we instead have the "constructor" for `ItemMaker` take a reference to
an `FdMonitor` instance and directly add itself to the monitor's set, meaning we
don't need to move the item out of the `ItemMaker` in order to add it to the
`FdMonitor` set later.
We were only using their ffi implementations which are automatically
exported/public, but the actual functions we would need if we were to use
FdMonitor and co. in native rust code were either private or missing convenient
wrappers.
The existing code is kept, but a rusty version of these functions is added for
code that needs them.
These should only be temporarily used when porting 1-to-1 from C++; we should
use the std library's `read()` and `write_all()` methods instead in the future.
By extracting the equivalent of i32::cmp() into its own const function,
it becomes a lot easier to see what is happening and the logic can be
more direct.
These will be used in the parser.
Maybe this type should be a struct with boolean fields. The current way has
the upside that the usage is exactly the same as in C++.
For some reason this error is triggered by tests after the Rust port of
ast.cpp. Might want to get to the bottom of this but moving it back
to match the original C++ logic fixes it.
This is one of the few warnings we disable due to false positives. Let's also
disable it in the preprocessing steps needed for the Rust build.
Other warnings we ignore are -Wno-address -Wunused-local-typedefs and
-Wunused-macros. I didn't add them here because I don't expect that they
will be triggered by the headers we give to cxx.
Prior to this fix, the Rust FLOG output was regressed from C++, because
it put quotes around strings. However if we used Display, we would fail
to FLOG non-display types like ThreadIDs.
There is apparently no way in Rust to write a function which formats a
value preferentially using Display, falling back to Debug.
Fix this by introducing two new traits, FloggableDisplay and
FloggableDebug. FloggableDisplay is implemented for all Display types,
and FloggableDebug can be "opted into" for any Debug type:
impl FloggableDebug for MyType {}
Both traits have a 'to_flog_str' function. FLOG brings them both into
scope, and Rust figures out which 'to_flog_str' gets called.
* wutil: Rewrite `wrealpath` in Rust
* Reduce use of FFI types in `wrealpath`
* Addressed PR comments regarding allocation
* Replace let binding assignment with regular comparison
More ugliness with types that cxx bridge can't recognize as being POD. Using
pointers to get/set `termios` values with an assert to make sure we're using
identical definitions on both sides (in cpp from the system headers and in rust
from the libc crate as exported).
I don't know why cxx bridge doesn't allow `SharedPtr<OpaqueRustType>` but we can
work around it in C++ by converting a `Box<T>` to a `shared_ptr<T>` then convert
it back when it needs to be destructed. I can't find a clean way of doing it
from the cxx bridge wrapper so for now it needs to be done manually in the C++
code.
Types/values that are drop-in ready over ffi are renamed to match the old cpp
names but for types that now differ due to ffi difficulties I've left the `_ffi`
in the function names to indicate that this isn't the "correct" way of using the
types/methods.
We want to keep the cast because tv_sec is not always 64 bits, see b5ff175b4
(Fix timer.rs cross-platform compilation, 2023-02-14).
It would be nice to avoid the clippy exemption, perhaps using something like
#[cfg(target_pointer_width = "32")]
let seconds = val.tv_sec as i64;
#[cfg(not(target_pointer_width = "32"))]
let seconds = val.tv_sec;
but I'm not sure if "target_pointer_width" is the right criteria.
Upsizing to `usize` from `i32` doesn't work if `usize` is only 32-bits.
I changed the code to use the `FromStr` impl on `i32`, but we could have also
just used `u64` instead of `i32`.
Also, we should get in the habit of using the appropriate type aliases where
possible (`i32` should be `RawFd`).
We want to try and catch as much unexpected/non-deterministic behavior as we
can. We could run the CI explicitly in debug mode, but I think it makes sense to
always have overflow checks on in both debug/release modes everywhere, at least
for the duration of the codebase transition.
The mutex was being locked from the very start, before it was needed and
possibly before it would be needed.
Also rename the static global to stick to rust naming conventions.
Note that `once_cell::sync::Lazy<T>` actually internally uses its own lock
around the value, but in this case it's insufficient because `SmallRng` doesn't
implement `SeedableRng` so we can't reseed it with only an `&mut` reference and
must instead replace its value.
We probably *could* still use `Lazy<SmallRng>` directly and then rely on
`std::mem::swap()` to replace the contents of the shared global static without
reassigning the variable directly with a new `SmallRng` instance, but I'm not
sure that's a great idea. This is just a built-in, there's no real harm in
locking twice (especially while fish remains essentially single-threaded).
The old comments about using i128 logic were still there even though we are no
longer using that approach and the output type was very much misleadingly a u64
printed to the console (but via `%d` so it was ultimately shown as an i64). Be
explicit about the resulting being a valid i64 value before passing it to the
sprintf!() macro.
Also add comments about the safety of the final `unwrap()` operation.
Rust doesn't have __FUNCTION__ or __func__ (though you can hack around it with a
proc macro, but that will require a separate crate and slowing down compilation
times with heavy proc macro dependencies), so these are just regular functions
(at least for now). Rust's default stack trace on panic (even in release mode)
should be enough (and the functions themselves are inlined so the calling
function should be the second frame from the top, after the #[cold] panic
functions).
This is to allow us to verify some implementation details that aren't explicitly
documented in the rust standard library's documentation.
std::thread uses `pthread_create()` underneath the hood on *nix platforms, so
this *should* merely be a formality.
