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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I realised this dropped off my radar for a while, for various reasons, so I'm picking this back up in an effort to get a consensus and to land something. I'm not bothering uploading a diff at this point, but my most recent proposal was:

> When casting, use `static_cast`, `reinterpret_cast`, and `const_cast` rather than C-style casts. There are two exceptions to this:
>
> - When casting to void to suppress warnings about unused variables (as an alternative to ``[[maybe_unused]]``). Prefer C-style casts in this instance.
> - When casting integral types (including enums), prefer the prevailing style in the area being worked on.

@mehdi_amini pointed out that this doesn't provide a proper convergence point. Would there be broad acceptance if we permitted functional-style casts for safe integral types, but not regular C-style casts? This would allow code to (eventually) converge towards at least two points. Concrete proposal:

> When casting, use `static_cast`, `reinterpret_cast`, and `const_cast` rather than C-style casts. There are two exceptions to this:
>
> - When casting to void to suppress warnings about unused variables (as an alternative to ``[[maybe_unused]]``). Prefer C-style casts in this instance.
> - When casting between integral types (including enums that are not strongly-typed), functional-style casts are permitted as an alternative to `static_cast`.

I also think it makes sense to move this block to the paragraph below https://llvm.org/docs/CodingStandards.html#do-not-use-rtti-or-exceptions, so that `dynamic_cast` is referenced in close proximity.

Some thoughts about the second bullet point in this proposal:

- I've explicitly excluded strongly-typed enums from functional-style casts. In my experience, the point of strongly typed enums is that they're always referred to by name. Deviating from this may be appropriate in limited cases, but those cases probably want highlighting by virtue of a `static_cast`.
- I considered adding a clause to it to only permit functional-style casts for safe casts where there is no risk of loss of precision (i.e. as outlined here <https://reviews.llvm.org/D151187#4364070>). However, those cases are typically cases where I personally DO use functional-style casts, to show the loss-of-precision is intentional. I don't oppose using `static_cast` in those cases though, so if there is preference to change this, I'm happy to do so.

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151187