[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.
aaron.ballman added a comment. In https://reviews.llvm.org/D16008#1035948, @malcolm.parsons wrote: > In https://reviews.llvm.org/D16008#1035789, @aaron.ballman wrote: > > > Do you know why the CSA checker is still in alpha? > > > It isn't - https://reviews.llvm.org/D26768 moved it to optin. > > I don't know why https://reviews.llvm.org/D34275 didn't turn it on by default. Then I'm not certain the utility of this check given that you can call the CSA check from clang-tidy directly; however, I do like the idea of exposing the CSA check under the name cert-oop50-cpp. Repository: rL LLVM https://reviews.llvm.org/D16008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.
malcolm.parsons added a comment. In https://reviews.llvm.org/D16008#1035789, @aaron.ballman wrote: > Do you know why the CSA checker is still in alpha? It isn't - https://reviews.llvm.org/D26768 moved it to optin. I don't know why https://reviews.llvm.org/D34275 didn't turn it on by default. Repository: rL LLVM https://reviews.llvm.org/D16008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.
aaron.ballman added a comment. In https://reviews.llvm.org/D16008#1035683, @alexfh wrote: > If the CSA checker is still in alpha, I'd proceed with this check instead of > investing time in polishing the CSA implementation. Do you know why the CSA checker is still in alpha? As best I can tell, that check and this one are attempting to do the same work. I'm kind of worried about having competing functionalities because users have to guess as to which one is the "correct" one to use. Repository: rL LLVM https://reviews.llvm.org/D16008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.
alexfh added a comment. Aaron, WDYT? Repository: rL LLVM https://reviews.llvm.org/D16008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.
alexfh added a comment. If the CSA checker is still in alpha, I'd proceed with this check instead of investing time in polishing the CSA implementation. Repository: rL LLVM https://reviews.llvm.org/D16008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.
lebedev.ri added a comment. Herald added subscribers: llvm-commits, xazax.hun, mgorny, klimek. Any further thoughts here? I was slightly bitten by this recently, and i though that it already existed as a clang-tidy check (: Repository: rL LLVM https://reviews.llvm.org/D16008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.
hokein added a comment. In http://reviews.llvm.org/D16008#322811, @Eugene.Zelenko wrote: > This check is duplicate of clang-analyzer-alpha.cplusplus.VirtualCall. Oops... Didn't notice there is an implementation already. > From my point of view, Clang-tidy is better place, since such calls doesn't > depend of run-time paths. > > I think will be good idea to try to establish better functionality separation > between Clang/Clang-tidy/Clang Static Analyzer. Current situation looks like > different teams try to take everything coming to them without considering big > picture. This my impression based on including padding check in Clang Static > Analyzer. > > I may be wrong, but Clang seems even better place for such warnings. However clang-tidy has a ' clang-analyzer-*' check option to run the clang static analyzer check. I'm not sure whether it's worthwhile to move `VirtualCall` check to the module in clang-tidy. Repository: rL LLVM http://reviews.llvm.org/D16008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. This check is duplicate of clang-analyzer-alpha.cplusplus.VirtualCall. From my point of view, Clang-tidy is better place, since such calls doesn't depend of run-time paths. I think will be good idea to try to establish better functionality separation between Clang/Clang-tidy/Clang Static Analyzer. Current situation looks like different teams try to take everything coming to them without considering big picture. This my impression based on including padding check in Clang Static Analyzer. Repository: rL LLVM http://reviews.llvm.org/D16008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
