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


& thank you for doing so - I really would like it if there were more voices
to discuss these things. Often feels a bit too quiet when I try to have a
discussion about them & there aren't (m)any other voices/perspectives.


> but I'm more than happy to revert the change since
> your involvement and experience in the project is vastly superior to mine
> :o)
>

FWIW, you can probably find an old thread I tried to have about some
similar/related API quirks in the Clang Tooling codebase & I didn't get so
far trying to explain my perspective to another LLVM contributor... *shakes
head* my perspectives certainly aren't universally accepted, even across
the LLVM project.


> Anyway, I'm at home now, so I won't be able to do it before tomorrow.
>
> Cheers
> Andy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151203/4cf3a0d0/attachment.html>