Re: [PATCH v4] rust: str: add {make,to}_{upper,lower}case() to CString

[Danilo Krummrich]

On 2/20/24 16:04, Miguel Ojeda wrote:
> On Tue, Feb 20, 2024 at 1:03 PM Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
> > That's the worst rationale I could think of. Without further rationale what that
> > should mean and why this would be good, it's entirely meaningless.
>
> Probably whoever wrote that did not feel the need to explain further
> because it is the convention, but please feel free to open an issue/PR
> to Clippy about improving the wording of that text.

The rational for a convention can't be that it is a convention. Instead
it should be a convention for an objective reason.

> The convention itself, however, you will find way harder to change
> everywhere else.

I'm not saying that we should enforce it otherwise, I just think that we
should have objective reasons for restrictions.

> > Instead, I'd argue that keeping it gives kernel people, who necessarily need to
> > deal with both, Rust *and* C, more consistency in kernel code.
>
> That sounds to me like trying to keep consistency in style/formatting
> between two languages, which is something we have discussed quite a
> few times in the past.

No, I didn't say, nor did I mean, that we should align with C in general,
nor should it be enforced.

However, I also don't see why we should disallow it as long as there is
no objective reason to do so.

> We are keeping Rust code as idiomatic as possible, except where it may
> actually make sense to diverge for kernel reasons.
>
> But this one does not seem to be the case:
>
>    - It is inconsistent with most Rust code out there.
>    - It is inconsistent with all Rust kernel code.
>    - It is inconsistent with learning material, which kernel developers use too.>    - It introduces 2 ways for writing the same trivial thing.

That's actually what the language did already with early-return vs return at
the end of the function.

I admit that consistent inconsistency is also kinda consistent though. :-)

>    - Rust is a more expression-oriented language than C.

The language has various characteristics, maybe that's why it allows both?

> And, by the way, your patch does use both ways. Why aren't you
> explicit when it is a single expression too?

See above.

> > At least, this shouldn't be fatal IMHO.
>
> For some of the compiler-based (i.e. not Clippy) and that may make
> prototyping a bad experience, I could agree (e.g. like missing
> documentation is already a warning).
>
> But please note that patches must be warning free anyway, so it is not
> like this patch would have been OK.

Then it shouldn't be a warning either IMHO.

> > Similar story here. Why is it bad, and even *fatal*, to be explicit?
>
> This one is more arguable, and could be discussed.

That's great, although I really don't understand why you think this one is, but
the other one isn't. What's the difference?

> In fact, we planned
> going through some of the lints in a meeting to see, mostly, what
> extra lints could be enabled etc. You are welcome to join if that
> happens (I think Trevor wanted to drive that discussion).

Thanks for the invitation, I'm happy to join!

> > Again, not a great rationale, this is entirely subjective and might even depend
> > on the context of the project. Again, for kernel people who need to deal with Rust
> > *and* C continuously it might be better to be explicit.
>
> That is fine, but to decide on this like this, we need better examples
> and rationale than just "it might be better" (and please note that
> whatever Clippy says is not important, so complaining about their docs
> being lacking is not really an argument to change kernel code).

I agree, but I also think it should be the other way around. We should have good
examples and an objective rationale for things we restrict.

> Cheers,
> Miguel