Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

mgehre updated this revision to Diff 37155. mgehre marked an inline comment as done. mgehre added a comment. Add test for static_cast with const http://reviews.llvm.org/D13368 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

This revision was automatically updated to reflect the committed changes. Closed by commit rL250098: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast (authored by mgehre). Changed prior to commit: http://reviews.llvm.org/D13368?vs=37155=37164#toc Repository: rL LLVM

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

I think, it would be more consistent to have clang-tidy insert a link to the check documentation (on llvm.org) into each warning. The documentation pages already contain links to the relevant rules. We could even version documentation pages, if we think it's important (but afaiu,

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. On Tue, Oct 6, 2015 at 5:46 PM, Matthias Gehre wrote: > mgehre marked an inline comment as done. > mgehre added a comment. > > I cannot think of any way to

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

sbenza added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp:28 @@ +27,3 @@ + + auto P0 = static_cast(new Base()); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

On Tue, Oct 6, 2015 at 10:19 AM, Manuel Klimek wrote: > On Tue, Oct 6, 2015 at 4:18 PM Aaron Ballman > wrote: >> >> On Tue, Oct 6, 2015 at 10:15 AM, Manuel Klimek wrote: >> > On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman wrote: > aaron.ballman added a comment. > > In http://reviews.llvm.org/D13368#260672, @klimek wrote: > > > In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote: > > > > > This wasn't a comment on the rule so much as

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

On Tue, Oct 6, 2015 at 10:15 AM, Manuel Klimek wrote: > On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman > wrote: >> >> aaron.ballman added a comment. >> >> In http://reviews.llvm.org/D13368#260672, @klimek wrote: >> >> > In

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

On Tue, Oct 6, 2015 at 4:18 PM Aaron Ballman wrote: > On Tue, Oct 6, 2015 at 10:15 AM, Manuel Klimek wrote: > > On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman > > wrote: > >> > >> aaron.ballman added a comment. > >> > >> In

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

mgehre marked an inline comment as done. mgehre added a comment. I cannot think of any way to improve the usefulness of the diagnostic in the non-polymorphic case. For now, this patch has a link to the CppCoreGuidelines document in the docs/**/*.rst file. Also, this check is not enabled by

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

mgehre updated this revision to Diff 36664. mgehre added a comment. Remove trailing [cppcoreguidelines-pro-type-static-cast-downcast] on tests http://reviews.llvm.org/D13368 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

aaron.ballman added a comment. In http://reviews.llvm.org/D13368#260672, @klimek wrote: > In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote: > > > This wasn't a comment on the rule so much as a comment on the diagnostic > > not being very helpful.In this case, you're telling the

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

klimek added a subscriber: klimek. klimek added a comment. In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote: > This wasn't a comment on the rule so much as a comment on the diagnostic not > being very helpful.In this case, you're telling the user to not do something, > but it is

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

mgehre marked an inline comment as done. Comment at: clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp:53 @@ +52,3 @@ + } else { +diag(MatchedCast->getOperatorLoc(), "do not use static_cast to cast from base class to derived class."); + }

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

mgehre updated this revision to Diff 36567. mgehre marked 2 inline comments as done. mgehre added a comment. Simplify logic to check for BaseToDerived cast. Add test for object to reference cast. Fixed comments http://reviews.llvm.org/D13368 Files:

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

mgehre added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp:33 @@ +32,3 @@ + const auto *SourceDecl = SourceType->getPointeeCXXRecordDecl(); + if(!SourceDecl) +SourceDecl = SourceType->getAsCXXRecordDecl();

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp:33 @@ +32,3 @@ + const auto *SourceDecl = SourceType->getPointeeCXXRecordDecl(); + if(!SourceDecl) +SourceDecl = SourceType->getAsCXXRecordDecl();

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

mgehre updated this revision to Diff 36465. mgehre marked 3 inline comments as done. mgehre added a comment. Fixed comments. Only show fixit for polymorphic classes. Ignore template instantiations for now http://reviews.llvm.org/D13368 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp:33 @@ +32,3 @@ + + SubExpr = SubExpr->IgnoreParens(); + QualType SourceType = SubExpr->getType(); Will the type dependence check also look through

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

sbenza added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp:19 @@ +18,3 @@ + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to cast from base to derived. Use dynamic_cast instead. (C++ Core Guidelines, rule

[PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

mgehre created this revision. mgehre added reviewers: alexfh, sbenza, bkramer, aaron.ballman. mgehre added a subscriber: cfe-commits. mgehre added a dependency: D13313: [clang-tidy] new check cppcoreguidelines-pro-type-reinterpret-cast. This check flags all usages of static_cast, where a base