Quuxplusone added a comment. In D76572#1937558 <https://reviews.llvm.org/D76572#1937558>, @rsmith wrote:
> In D76572#1935791 <https://reviews.llvm.org/D76572#1935791>, @lebedev.ri > wrote: > > > Passing-by remark: > > > > > I wrote a Clang warning [not pictured] to diagnose any use of `T(x)` > > > which was not equivalent to `static_cast<T>(x)`. > > > > I'm not sure whether or not this will pass the bar for a clang diagnostic > > > I'd like to try it out on a larger codebase, but it sounds at least > potentially good to me. There's a simple syntactic workaround (use `(T)x` > instead of `T(x)`), and there's a high likelihood that the code doesn't mean > what the programmer intended. > > Does the warning catch the cases where the code is equivalent to > `static_cast<T>(x)` except that it ignores access? That seems like a really > good thing to warn on regardless of whether we warn on the general case. Yes, I verified with some simple test cases that it catches the equivalents of (1) reinterpret_cast, (2) const_cast, (3) const_cast plus static_cast, (4) upcast to private base, and (5) downcast to private base. However, the thing I wrote is 100% **not** ready for prime time. It basically just inserts a call to `TryStaticCast` from inside `CheckCXXCStyleCast`. Unfortunately `TryStaticCast` is one of these functions that Clang is full of, where it both "checks if X is possible" and also "does X" and also "prints error messages to stdout," all from within the same function. So my patch ends up being morally equivalent to just treating `T(x)` as `static_cast<T>(x)`; it emits some errors that shouldn't be there if it's supposed to just warn, and I have no idea how to shut up those errors except by completely refactoring `TryXXXXCast`. (E.g., it could take an extra parameter `Mode` that indicated whether to actually do the conversion, or just report whether it was possible, or emit notes about why it was impossible.) I wrestled unsuccessfully with similar issues when I did `-Wreturn-std-move`. I 100% want to see someone like you tackle this issue, but take my word for it, you **don't** want to start with the code I wrote the other day. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76572/new/ https://reviews.llvm.org/D76572 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
