aaron.ballman added a comment.
On Mon, Oct 5, 2015 at 6:14 PM, Matthias Gehre <m.ge...@gmx.de> wrote:
> 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.");
> + }
>
> ----------------
>
> aaron.ballman wrote:
>
> > What is the alternative the user should use in this instance?
>
>
> The alternative is to either
>
> 1. change the program logic to not require and downcast of a non-polymorphic
> type or
> 2. add NOLINT to notify your coworkers that this may be the spot where
> something goes wrong
>
> In the end, these rules together should eliminate some classes of undefined
> behavior and crashes. If the application still crashes, you should just need
> to look at the (hopefully few) places of NOLINT.
>
> Anyway, I didn't make the rules. See Herb Sutters answer here:
> https://github.com/isocpp/CppCoreGuidelines/issues/270
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 unclear how the user would structure their code to silence this
diagnostic. Perhaps there is no way to word this to give the user a clue, but
we should at least try. If I got this diagnostic as it is now, I would scratch
my head and quickly decide to ignore it.
> ================
> Comment at:
> clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp:33
> @@ +32,3 @@
> + const auto *SourceDecl = SourceType->getPointeeCXXRecordDecl();
> + if(!SourceDecl)
> + SourceDecl = SourceType->getAsCXXRecordDecl();
>
> ----------------
>
> aaron.ballman wrote:
>
> > In the event it's not a pointer or a reference, why are you getting the
> > source as a value type?
>
>
> Source type could be no pointer nor ref like in:
>
> Base B0;
> auto R0 = static_cast<Derived&>(B0);
Ah, interesting! I was guessing that case would have had an lvalue reference
type for B0, but I'm guessing it's already gone through lvalue to rvalue
conversion before hitting the cast node (or my intuition is just off).
================
Comment at:
test/clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp:39
@@ +38,3 @@
+ auto PP0 = static_cast<PolymorphicDerived*>(new PolymorphicBase());
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to
downcast from a base to a derived class; use dynamic_cast instead
[cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-FIXES: auto PP0 = dynamic_cast<PolymorphicDerived*>(new
PolymorphicBase());
----------------
No need to have the [cppcoreguidelines-pro-type-static-cast-downcast] part of
the message on anything but the first diagnostic in the file. (This reduces
clutter, but we test it one time just to make sure it's there).
http://reviews.llvm.org/D13368
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
