[PATCH] D151187: [doc] Add casting style preference to coding standards

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D151187#4421303 <https://reviews.llvm.org/D151187#4421303>, @barannikov88 wrote:

>> ! In D151187#4420876 <https://reviews.llvm.org/D151187#4420876>, @aaron.ballman wrote:
>>  I like that phrasing, thank you! Do others have opinions on using this wording over the existing wording?
>
> Yup, it looks kind of redundant because at the very beginning of the coding standards it is already spelled, in bold:
>
>> If you are extending, enhancing, or bug fixing already implemented code, use the style that is already being used so that the source is uniform and easy to follow.

Essentially, the second bullet point is saying that integrals are specifically excluded from the rule, so fall back to the default. Whilst it's redundant, I think it's important to say it explicitly here. Plus, if you are writing some new code, or it's in an area where there is no prevailing style, and this basically leaves the door open to "choose what you want", rather than prescribing that `static_cast` is used. At least, that's how I'd interpret it.

> And it still forbids functional style casts. I'd like them to explicitly be allowed for constructing objects, as if the type being cast to was a class.
> In D152098 <https://reviews.llvm.org/D152098> I introduce a typedef MCRegUnit and use functional style cast to construct it from integer, like this:
> `Val = MCRegUnit(*++I);`
> This violates the suggested wording; I'm supposed to use static_cast here. But I plan to turn the typedef into a class in the future, and don't want to use static_cast for calling class' constructor, it is unnatural.
> I'd also allow functional-style casts for constructing enums from integers. static_cast isn't safer here, only more noisy.
> I'm fine with static_cast in all other cases, and I also support forbidding C-style casts completely.
>
> Unfortunately, my English is too poor to suggest a wording that would take these cases into account.

A typdefed integral is still an integral type, so the rule is to follow the prevailing style. If functional style casts are already in use elsewhere in that area of code, then the code you've highlighted would be fine. Once you turn it into a class, it's no longer a functional-style cast - it's simply calling the class constructor, so this coding standard isn't applicable. (If the issue is more that you'd be forced to use a static_cast initially and then the transition to a class results in an unnecessary static_cast, I think it would be fine to argue that you're ignoring the style guide because a pending change will work better, and I think that rule applies for all parts of the style guide).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151187/new/

https://reviews.llvm.org/D151187