[patch] Make "C++98 requires an accessible copy constructor" warning DefaultIgnore

Richard Smith richard at metafoo.co.uk
Wed Sep 17 15:43:33 PDT 2014


Index: test/SemaCXX/cxx98-compat.cpp
===================================================================
--- test/SemaCXX/cxx98-compat.cpp (revision 217313)
+++ test/SemaCXX/cxx98-compat.cpp (working copy)
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat -verify %s
-// RUN: %clang_cc1 -fsyntax-only -std=c++1y -Wc++98-compat -verify %s
-DCXX14COMPAT
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat
-Wc++98-compat-bind-to-temporary-copy -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -Wc++98-compat
-Wc++98-compat-bind-to-temporary-copy -verify %s -DCXX14COMPAT

Please move the relevant tests to test/SemaCXX/cxx98-compat-pedantic.cpp
instead of adding the warning flag here.

Otherwise, LGTM, thanks!

On Sun, Sep 14, 2014 at 3:11 PM, Nico Weber <thakis at chromium.org> wrote:

> Ah, that makes sense. Here's another version that
> moves CXX98CompatBindToTemporaryCopy over and
> makes ext_rvalue_to_reference_temp_copy_no_viable an Extension. (For some
> reason, I had read your second mail as "ou should also move the existing
> -Wc++98-compat warnings over" and thought you meant all of them.)
>
> On Sat, Sep 6, 2014 at 8:14 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Sat, Sep 6, 2014 at 5:31 PM, Nico Weber <thakis at chromium.org> wrote:
>>
>>> On Sat, Sep 6, 2014 at 5:02 PM, Nico Weber <thakis at chromium.org> wrote:
>>>
>>>> On Sat, Sep 6, 2014 at 4:38 PM, Richard Smith <richard at metafoo.co.uk>
>>>> wrote:
>>>>
>>>>> -Wc++98-compat doesn't make sense here: that warning group is for
>>>>> warning on code that is valid in the current language but not in C++98.
>>>>>
>>>>
>>>> The same is true for warn_cxx98_compat_enumerator_list_comma (which is
>>>> in CXX98CompatPedantic) too though, right?
>>>>
>>>
>> That's not how the cxx98_compat warnings work. This is what we do there:
>>
>>       if (!getLangOpts().C99 && !getLangOpts().CPlusPlus11)
>>         Diag(CommaLoc, getLangOpts().CPlusPlus ?
>>                diag::ext_enumerator_list_comma_cxx :
>>                diag::ext_enumerator_list_comma_c)
>>           << FixItHint::CreateRemoval(CommaLoc);
>>       else if (getLangOpts().CPlusPlus11)
>>         Diag(CommaLoc, diag::warn_cxx98_compat_enumerator_list_comma)
>>           << FixItHint::CreateRemoval(CommaLoc);
>>
>> Note that we never produce a -Wc++98-compat warning in C++98 mode. And we
>> do a similar thing for this warning, but it's a little more complex because
>> we can't go through the motions of creating the temporary in C++11 mode,
>> since that might trigger template instantiations we're not allowed to
>> perform etc.
>>
>> …anyhoo, here's a version that just makes this warning an Extension.
>>>
>>
>> You should move over ext_rvalue_to_reference_temp_copy_no_viable too. It
>> doesn't make sense to downgrade only one of these two to an extension
>> (either we ignore the temporary copy as an extension or we don't; we
>> shouldn't be halfway between).
>>
>>
>>> I didn't move CXX98CompatBindToTemporaryCopy to
>>> CXX98CompatPedanticbecause that seems like an unrelated change,
>>>
>>
>> It isn't: -Wc++98-compat-pedantic is supposed to warn on things that
>> would warned about in -pedantic C++98 mode. Since you're moving this
>> warning from ExtWarn to Extension, that means the corresponding
>> -Wc++98-compat warning should move to -Wc++98-compat-pedantic.
>>
>> and because it contradicts the comment above CXX98CompatPedantic:
>>>
>>> // Warnings for C++11 features which are Extensions in C++98 mode.
>>>
>>
>> You're making the C++11 "ignore the temporary copy when binding a
>> reference to a temporary of the same type" feature an Extension in C++98
>> mode, so -Wc++98-compat-bind-to-temporary-copy belongs in the
>> -Wc++98-compat-pedantic group in C++11 mode. I don't see a contradiction
>> with the comment, but maybe we could clarify it somehow?
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140917/8984c6fa/attachment.html>


More information about the cfe-commits mailing list