On 16.07.24 09:44, Miguel Ojeda wrote:
On Mon, Jul 15, 2024 at 6:12 PM Michal Rostecki <vadorovsky@xxxxxxxxx> wrote:
@@ -71,11 +75,11 @@ macro_rules! kunit_assert {
//
// This mimics KUnit's failed assertion format.
$crate::kunit::err(format_args!(
- " # {}: ASSERTION FAILED at {FILE}:{LINE}\n",
+ " # {:?}: ASSERTION FAILED at {FILE:?}:{LINE:?}\n",
$name
));
$crate::kunit::err(format_args!(
- " Expected {CONDITION} to be true, but is false\n"
+ " Expected {CONDITION:?} to be true, but is false\n"
));
The only practical difference in switching from `Display` to `Debug`
here is that the fallback kunit error messages are going to include
quotation marks around conditions, files and lines.
That is a fairly important difference -- the messages are intended to
match the C KUnit ones.
Especially the file:line notation -- I don't think a user would expect
to have quotes there (regardless of KUnit).
In general, even if we didn't need it right now, I think it is
something we will need sooner or later.
Alright, I will go with Trevor's suggestion and provide a `display()`
method via an extension trait.
wording. My general point is that I've never seen `&mut str` being
exposed in any core/std API to the external user, mutation usually
implies usage of an owned String.
Not sure what you mean by exposed, but even if `&mut str`'s methods do
not count (used via `String`), there is also
`from_utf8_unchecked_mut()` that returns one, which is essentially the
same idea as what we had here.
So I am not sure about the "The rule of Rust std" part in the new
commit messages. And, to be clear, while the Rust standard library is
a good reference to look into, sometimes we want/need to do things
differently anyway (which is not really the case here given
`from_utf8_unchecked_mut()`, I would say).
In this case, regardless of the standard library, personally I would
have preferred to have a non-public function, but still have it (and
properly documented), rather than open code the `unsafe` block with
the casts.
Fair enough. I will provide `from_utf8_unchecked_mut()` as a part of
`CStrExt` in the next version.
I think the best solution would be leaving `c_str` macro for that. The
reason why I removed it is that the GitHub issue[0] mentions its
removal. But for that case, I think it makes sense to leave it. What do
you think?
Perhaps the issue was only taking into account the C string literal
case? Benno may know more.
Generally speaking, replacing a clean line with a bigger `unsafe`
block is something to be avoided.
Maybe a `c_stringify!` is what we need :)
`stringify!` is not the only case where I ended up using `c_str!`. After
addressing Björn's suggestion about taking Rust strings as arguments in
`new_mutex!`, `new_condvar!` etc., `optional_name!` is also using
`c_str!` in the following way:
macro_rules! optional_name {
() => {
$crate::c_str!(::core::concat!(::core::file!(), ":",
::core::line!()))
};
($name:literal) => {
$crate::c_str!($name)
};
}
So I think that leaving `c_str!` still makes sense, unless you have
other suggestions, which are still easily applicable there. :)
Cheers,
Miguel
Cheers,
Michal
