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

[Aaron Ballman via cfe-commits]

On Tue, Oct 6, 2015 at 10:19 AM, Manuel Klimek <klimek at google.com> wrote:
> On Tue, Oct 6, 2015 at 4:18 PM Aaron Ballman <aaron.ballman at gmail.com>
> wrote:
>>
>> On Tue, Oct 6, 2015 at 10:15 AM, Manuel Klimek <klimek at google.com> wrote:
>> > On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman <aaron.ballman at gmail.com>
>> > 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 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.
>> >> >
>> >> >
>> >> > The cpp core guidelines are written in a way that they should be
>> >> > referenceable by links - do we want to add links to the relevant
>> >> > portions of
>> >> > the core guidelines from the clang-tidy checks?
>> >>
>> >>
>> >> I'd be hesitant to do that. It would add a lot of verbiage to
>> >> diagnostics
>> >> that are likely to be chatty, and if the links ever go dead mid-release
>> >> cycle for us, we're stuck looking bad with no way to fix it. CERT's
>> >> guidelines are also linkable in the same fashion (as would be
>> >> hypothetical
>> >> checks for MISRA, JSF, etc), and I would have the same hesitation for
>> >> those
>> >> as well due to the potential dead link issue.
>> >>
>> >> I think that having the links within the user-facing documentation is a
>> >> must-have though (and something we've been pretty good about thus far)
>> >> because those can be updated live from ToT. So perhaps a permanent
>> >> short
>> >> link to our own documentation might be useful (if a bit obtuse since
>> >> our
>> >> docs mostly just point to other docs elsewhere)? I'd still be a bit
>> >> worried
>> >> about expired short links or something, but maybe we already host a
>> >> service
>> >> for this sort of thing?
>> >
>> >
>> > I'll postulate that a) github will not go away anytime soon and b)
>> > github
>> > will have a hard time changing their link structure so linking into
>> > revision
>> > N in branch M doesn't work any more.
>> > Thus, I think if we link into the github release of the core
>> > guildelines,
>> > we'll be fine.
>>
>> Github's structure and stability isn't what I'm worried about. The C++
>> Core Guidelines internal structure is what I am worried about. They
>> currently use anchors to navigate around CppCoreGuidelines.md, and
>> those anchor names may or may not be stable. Even with the best of
>> intentions on stability, links change.
>
>
> Can't we link it to one specific version in time, and update the base
> revision when we did QA on the links?

Yes, we could link it to the git hash, but then we would likely want
to shorten the links for display to the user (perhaps as a service off
llvm.org so we can control the rewrites with more granularity).

I think this is a good idea that should have some broader community
discussion to iron out the details.

~Aaron