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

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added a comment.

In D151187#4365454 <https://reviews.llvm.org/D151187#4365454>, @mehdi_amini wrote:

>> Someone would have to test over a large corpus of code to see what the false positive vs true positive rate looks like. My intuition is that it would have too many false positives to be enabled by default, so it wouldn't meet the bar for a clang diagnostic.
>
> Don't we have many clang-diagnostics that aren't enabled by default? I thought that was a given that people can opt-in specific warnings in this category.
> What if we got LLVM entirely clean for such a warning: would it be good enough signal that it can be useful?

We do have off-by-default warnings but those tend to mostly be: historical warnings we likely would never add today, pedantic warnings for extensions/future standards compatibility, or they're for extremely one-off situations (like serious performance concerns that aren't also correctness concerns). We have evident that off-by-default warnings basically do not get enabled often enough to warrant adding them to the tool. So getting LLVM warning free for this wouldn't actually move the needle all that much.

In D151187#4365471 <https://reviews.llvm.org/D151187#4365471>, @tschuett wrote:

> I wondering what you mean by false positives? You know the type on the left and right of the cast. There should be some rules to find fishy casts. You want to cast a struct to a bool?

I mean: a situation where the existing C-style cast is as correct as the suggested named cast replacement (basically, where the switch between cast styles would be a noop and so users are likely to find the diagnostic too opinionated).

`(void)some_value_to_ignore;` is identical to `static_cast<void>(some_value_to_ignore);` per spec, so any diagnostic of that variety would be a false positive, for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151187