r216267 - Fixing MSVC warning: "warning C4373: 'blah': virtual function overrides 'blah', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers."

[Aaron Ballman]

On Fri, Aug 22, 2014 at 1:50 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Fri, Aug 22, 2014 at 6:26 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> Author: aaronballman
>> Date: Fri Aug 22 08:26:37 2014
>> New Revision: 216267
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=216267&view=rev
>> Log:
>> Fixing MSVC warning: "warning C4373: 'blah': virtual function overrides 'blah', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers."
>>
>> This warning is basically useless because the "previous versions" being referred to is MSVC 2005 and earlier, and we obviously do not care about them. However, this warning isn't particularly chatty (I don't recall ever seeing it previously), and it has the opportunity to point out cases where the cv-qualifiers differ unintentionally (like this case), so I am leaving it enabled for now.
>
> I'll put in a vote to disable it - the mismatch isn't at all
> important. It's an implementation detail - if someone wants to make
> their parameters locally const to ensure they don't accidentally
> modify them, that's OK.
>
> (but, I agree - I doubt anyone made the conscious choice to do that in
> this instance - probably just muscle memory of "const char"[*])
>
> One fewer thing to have the MSVC build break because of.

My default view is that all warnings should remain enabled unless
there's actually a problem with leaving it that way. Given that this
is the first time I've seen that warning in over three years and it
just pointed out some sketchy (but valid!) code, I'm not convinced
it's worth disabling. However, my opinion on the matter isn't terribly
strong, either.

To be clear, the MSVC build doesn't break because of this warning
(though I would love an MSVC bot that compiles with /WX to treat
warnings as errors, especially since we're warning-free already).

~Aaron