<div dir="ltr"><div>Index: test/SemaCXX/cxx98-compat.cpp</div><div>===================================================================</div><div>--- test/SemaCXX/cxx98-compat.cpp<span class="" style="white-space:pre"> </span>(revision 217313)</div><div>+++ test/SemaCXX/cxx98-compat.cpp<span class="" style="white-space:pre"> </span>(working copy)</div><div>@@ -1,5 +1,5 @@</div><div>-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat -verify %s</div><div>-// RUN: %clang_cc1 -fsyntax-only -std=c++1y -Wc++98-compat -verify %s -DCXX14COMPAT</div><div>+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat -Wc++98-compat-bind-to-temporary-copy -verify %s</div><div>+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -Wc++98-compat -Wc++98-compat-bind-to-temporary-copy -verify %s -DCXX14COMPAT</div><div><br></div><div>Please move the relevant tests to test/SemaCXX/cxx98-compat-pedantic.cpp instead of adding the warning flag here.</div><div><br></div><div>Otherwise, LGTM, thanks!</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Sep 14, 2014 at 3:11 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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 "<span style="font-family:arial,sans-serif;font-size:13px">ou should also move the existing -Wc++98-compat warnings over" and thought you meant all of them.)</span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Sep 6, 2014 at 8:14 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Sat, Sep 6, 2014 at 5:31 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Sat, Sep 6, 2014 at 5:02 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Sat, Sep 6, 2014 at 4:38 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><p dir="ltr">-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.</p></blockquote><div><br></div></span><div>The same is true for warn_cxx98_compat_enumerator_list_comma (which is in CXX98CompatPedantic) too though, right?</div></div></div></div></blockquote></span></div></div></div></blockquote><div><br></div></span><div>That's not how the cxx98_compat warnings work. This is what we do there:</div><div><pre style="color:rgb(0,0,0)"> 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);</pre></div><div>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.</div><span><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>…anyhoo, here's a version that just makes this warning an Extension.</div></div></div></div></blockquote><div><br></div></span><div>You should move over <span style="color:rgb(0,0,0)">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).</span></div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I didn't move CXX98CompatBindToTemporaryCopy to CXX98CompatPedanticbecause that seems like an unrelated change,</div></div></div></div></blockquote><div><br></div></span><div>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.</div><span><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>and because it contradicts the comment above CXX98CompatPedantic:</div><div><br></div><div><div>// Warnings for C++11 features which are Extensions in C++98 mode.</div></div></div></div></div></blockquote><div><br></div></span><div>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?<br></div></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>