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

[David Blaikie via llvm-commits]

On Thu, Dec 3, 2015 at 11:01 AM, Andy Gibbs <andyg1001 at hotmail.co.uk> wrote:

> 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.


Sounds like a slippery slope fallacy, no?

We know there's a flaw in this diagnostic here, so we disable it. In some
cases we may find the GCC diagnostic is superior and that we should fix
Clang instead. In many cases the diagnostics have obvious behavior and
either initially, or after some iteration, they match behavior between the
two compilers.

(for another example of a divergence, where we disable the GCC diagnostic,
see the overloaded-virtual warning disabling, if I recall correctly)


> 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)


Yep, there's a fine line between premature optimization and premature
pessimization. The same line that causes us to pass std::strings by const
ref (or StringRef) rather than by value, for example. And not everyone
draws the line in the same place, for sure.


>
>
> 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.


Sure sure, no rush.

- Dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151203/98507fe9/attachment.html>