[llvm] r254592 - Fix class SCEVPredicate has virtual functions and accessible non-virtual destructor.

[Andy Gibbs via llvm-commits]

On Thursday, December 3, 2015 6:55PM, David Blaikie wrote:
> We still have Clang that (arguably) correctly implements the warning 
> without
> the friend exception that GCC implements.
>
> My basic bar would be: Either we consider GCC's behavior correct and we 
> fix
> Clang's warning to do the same (& keep your code change). Or we consider
> GCC's behavior to be incorrect and we undo this change and disable GCC's
> warning.

Ok, but the downside of this approach is that ultimately (a bit hyperbolic
I grant you) we end up disabling all of GCC's warnings and trust only to
clang's.  I think this is a dangerous approach - even with all the 
confidence
I have in clang getting things right.

> My usual reason to push back against changes like this is the precedent -
> I don't want us to keep making sub-optimal changes to placate a warning.
> Death by a thousand cuts & all that (though there's probably only tens of
> cases of non-virtual dtors in polymorphic hierarchies across LLVM and 
> Clang,
> to be fair)

Heh, maybe (given the precise impact here) tens of thousands!  But I take
your point.  I certainly wouldn't dare get into an argument about anything 
to
do with "optimal" -- I have a coding partner who delights in shouting
"premature optimisation" at me at the slightest provocation :o)

> I'd rather discuss these things & come to an understanding/agreement, but
> if a request is simpler, I think this change should be reverted and, if
> you'd like to keep the GCC build warning-clean, a patch to disable this
> warning would be acceptable. Clang's warnings will continue to catch the
> other cases & self-hosting buildbots will flag them soon enough.

Well, I've stated my case, but I'm more than happy to revert the change 
since
your involvement and experience in the project is vastly superior to mine 
:o)

Anyway, I'm at home now, so I won't be able to do it before tomorrow.

Cheers
Andy