[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp:342
+  auto w = new int(x);
+}
----------------
Quuxplusone wrote:
> What about
> ```
> template<class T> T foo(int i) { return T(i); }
> int main() {
>     foo<std::vector<int>>(); // valid, OK(!)
>     foo<double>(); // valid, not OK
> }
> ```
> What about
> ```
> struct Widget { Widget(int); };
> using T = Widget;
> using U = Widget&;
> int i = 42;
> Widget t = T(i);  // valid, OK?
> Widget u = U(i);  // valid C++, should definitely not be OK
> ```
> https://quuxplusone.github.io/blog/2020/01/22/expression-list-in-functional-cast/
Good point, thanks! I've added the first one to the unit test.

Regarding the second check, I'm not sure if it's the scope of this check. This check does not care whether the constructor of the class is implicit or not - if you use a class type, then you are calling the constructor so it's fine. Same goes when it's a reference - in my opinion this check is not concerned with that.

I definitely see the problems that can arise from the example that you posted, but maybe it fits better as a separate check in the `bugprone` category? This check (`google-readability-casting`) is focused only about avoiding C-style casting, i.e. it's a readability/style/modernize matter IMO. If cpplint is not diagnosing that, I don't think this check should either.


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

https://reviews.llvm.org/D114427