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

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added inline comments.


================
Comment at: llvm/docs/CodingStandards.rst:1270
+When casting, use ``static_cast`` and ``reinterpret_cast`` rather than C-style
+casts. The sole exception to this is when casting to void to suppress warnings
+about unused variables, when C-style casts should be preferred instead:
----------------
jhenderson wrote:
> aaron.ballman wrote:
> > FWIW, I think it would be reasonable to also mention use of `[[maybe_unused]]` as a potential alternative to casting to `void`, but without prejudice as to which approach to use.
> I feel like any reference to `[[maybe_unused]]` belongs either in its own section (which I don't propose to add at this time) or in the section above where asserts are discussed, as that talks about casting to void to suppress the warnings. What I want to achieve here is to ensure the rule for preferring C++-style doesn't apply to that specific case. `[[maybe_unused]]` is an alternative solution, but isn't relevant to casting.
SGTM!


================
Comment at: llvm/docs/CodingStandards.rst:1274-1276
+  int x = 42;
+  long *y = reinterpret_cast<long *>(&x);
+  (void)y;
----------------
jhenderson wrote:
> aaron.ballman wrote:
> > How about a more realistic example along the lines of:
> > ```
> > int check_value = doSomeWorkWithSideEffects();
> > assert(check_value == 42 && "the assert is removed in NDEBUG builds");
> > (void)check_value;
> > ```
> I want to avoid essentially repeating the example or explanation from the previous section about asserts. I'm tempted to drop the void cast example entirely.
Ah you're right, we do have that example directly above. I think it'd be fine to drop the example here then. I was mostly concerned that the existing example seemed unmotivating as to why you'd need a cast to void in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151187