<div dir="ltr"><div><br>Hans,<br><br>Thank you for fixing the test!<br><br></div><div>Thanks</div><div><br></div><div>Galina<br></div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 11, 2017 at 10:59 AM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@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">I've committed a fix to pacify the test in r320405.</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 6, 2017 at 12:27 PM, Galina Kistanova via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.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">Hello Richard,<br><br>This commit broke the tests on the builder:<br><br><a href="http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/6598" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/llvm-clang-x86_64-expensiv<wbr>e-checks-win/builds/6598</a><br><br>. . .<br>Failing Tests (1):<br> Clang :: SemaCXX/warn-enum-compare.cpp<br><br>Please have a look?<br><br>Thanks<br><br>Galina<br></div><div class="gmail_extra"><br><div class="gmail_quote"><span>On Tue, Dec 5, 2017 at 7:00 PM, Richard Smith via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br></span><div><div class="m_-6697281527724097482h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Tue Dec 5 19:00:51 2017<br>
New Revision: 319875<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=319875&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=319875&view=rev</a><br>
Log:<br>
Fix a bunch of wrong "tautological unsigned enum compare" diagnostics in C++.<br>
<br>
An enumeration with a fixed underlying type can have any value in its<br>
underlying type, not just those spanned by the values of its enumerators.<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaCheckin<wbr>g.cpp<br>
cfe/trunk/test/Sema/tautologic<wbr>al-unsigned-enum-zero-compare.<wbr>cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaCheckin<wbr>g.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=319875&r1=319874&r2=319875&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Sema/SemaC<wbr>hecking.cpp?rev=319875&r1=3198<wbr>74&r2=319875&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/SemaCheckin<wbr>g.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaCheckin<wbr>g.cpp Tue Dec 5 19:00:51 2017<br>
@@ -8260,11 +8260,12 @@ struct IntRange {<br>
} else if (const EnumType *ET = dyn_cast<EnumType>(T)) {<br>
// For enum types in C++, use the known bit width of the enumerators.<br>
EnumDecl *Enum = ET->getDecl();<br>
- // In C++11, enums without definitions can have an explicitly specified<br>
- // underlying type. Use this type to compute the range.<br>
- if (!Enum->isCompleteDefinition()<wbr>)<br>
+ // In C++11, enums can have a fixed underlying type. Use this type to<br>
+ // compute the range.<br>
+ if (Enum->isFixed()) {<br>
return IntRange(C.getIntWidth(QualTyp<wbr>e(T, 0)),<br>
!ET->isSignedIntegerOrEnumera<wbr>tionType());<br>
+ }<br>
<br>
unsigned NumPositive = Enum->getNumPositiveBits();<br>
unsigned NumNegative = Enum->getNumNegativeBits();<br>
<br>
Modified: cfe/trunk/test/Sema/tautologic<wbr>al-unsigned-enum-zero-compare.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp?rev=319875&r1=319874&r2=319875&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/Sema/taut<wbr>ological-unsigned-enum-zero-co<wbr>mpare.cpp?rev=319875&r1=319874<wbr>&r2=319875&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Sema/tautologic<wbr>al-unsigned-enum-zero-compare.<wbr>cpp (original)<br>
+++ cfe/trunk/test/Sema/tautologic<wbr>al-unsigned-enum-zero-compare.<wbr>cpp Tue Dec 5 19:00:51 2017<br>
@@ -2,11 +2,11 @@<br>
// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s<br>
// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enu<wbr>m-zero-compare -verify %s<br>
<br>
-// Okay, this is where it gets complicated.<br>
-// Then default enum sigdness is target-specific.<br>
-// On windows, it is signed by default. We do not want to warn in that case.<br>
-<br>
int main() {<br>
+ // On Windows, all enumerations have a fixed underlying type, which is 'int'<br>
+ // if not otherwise specified, so A is identical to C on Windows. Otherwise,<br>
+ // we follow the C++ rules, which say that the only valid values of A are 0<br>
+ // and 1.<br>
enum A { A_foo = 0, A_bar, };<br>
enum A a;<br>
<br>
@@ -87,21 +87,23 @@ int main() {<br>
<br>
if (c < 0)<br>
return 0;<br>
- if (0 >= c) // expected-warning {{comparison 0 >= 'enum C' is always true}}<br>
+ if (0 >= c)<br>
return 0;<br>
- if (c > 0) // expected-warning {{comparison 'enum C' > 0 is always false}}<br>
+ if (c > 0)<br>
return 0;<br>
if (0 <= c)<br>
return 0;<br>
- if (c <= 0) // expected-warning {{comparison 'enum C' <= 0 is always true}}<br>
+ if (c <= 0)<br>
return 0;<br>
if (0 > c)<br>
return 0;<br>
if (c >= 0)<br>
return 0;<br>
- if (0 < c) // expected-warning {{0 < 'enum C' is always false}}<br>
+ if (0 < c)<br>
return 0;<br>
<br>
+ // FIXME: These diagnostics are terrible. The issue here is that the signed<br>
+ // enumeration value was promoted to an unsigned type.<br>
if (c < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}<br>
return 0;<br>
if (0U >= c)<br>
@@ -121,21 +123,23 @@ int main() {<br>
#elif defined(SIGNED)<br>
if (a < 0)<br>
return 0;<br>
- if (0 >= a) // expected-warning {{comparison 0 >= 'enum A' is always true}}<br>
+ if (0 >= a)<br>
return 0;<br>
- if (a > 0) // expected-warning {{comparison 'enum A' > 0 is always false}}<br>
+ if (a > 0)<br>
return 0;<br>
if (0 <= a)<br>
return 0;<br>
- if (a <= 0) // expected-warning {{comparison 'enum A' <= 0 is always true}}<br>
+ if (a <= 0)<br>
return 0;<br>
if (0 > a)<br>
return 0;<br>
if (a >= 0)<br>
return 0;<br>
- if (0 < a) // expected-warning {{comparison 0 < 'enum A' is always false}}<br>
+ if (0 < a)<br>
return 0;<br>
<br>
+ // FIXME: As above, the issue here is that the enumeration is promoted to<br>
+ // unsigned.<br>
if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}<br>
return 0;<br>
if (0U >= a)<br>
@@ -189,19 +193,19 @@ int main() {<br>
<br>
if (c < 0)<br>
return 0;<br>
- if (0 >= c) // expected-warning {{comparison 0 >= 'enum C' is always true}}<br>
+ if (0 >= c)<br>
return 0;<br>
- if (c > 0) // expected-warning {{comparison 'enum C' > 0 is always false}}<br>
+ if (c > 0)<br>
return 0;<br>
if (0 <= c)<br>
return 0;<br>
- if (c <= 0) // expected-warning {{comparison 'enum C' <= 0 is always true}}<br>
+ if (c <= 0)<br>
return 0;<br>
if (0 > c)<br>
return 0;<br>
if (c >= 0)<br>
return 0;<br>
- if (0 < c) // expected-warning {{0 < 'enum C' is always false}}<br>
+ if (0 < c)<br>
return 0;<br>
<br>
if (c < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}<br>
@@ -221,21 +225,22 @@ int main() {<br>
if (0U < c)<br>
return 0;<br>
#else<br>
+ // expected-no-diagnostics<br>
if (a < 0)<br>
return 0;<br>
- if (0 >= a) // expected-warning {{comparison 0 >= 'enum A' is always true}}<br>
+ if (0 >= a)<br>
return 0;<br>
- if (a > 0) // expected-warning {{comparison 'enum A' > 0 is always false}}<br>
+ if (a > 0)<br>
return 0;<br>
if (0 <= a)<br>
return 0;<br>
- if (a <= 0) // expected-warning {{comparison 'enum A' <= 0 is always true}}<br>
+ if (a <= 0)<br>
return 0;<br>
if (0 > a)<br>
return 0;<br>
if (a >= 0)<br>
return 0;<br>
- if (0 < a) // expected-warning {{comparison 0 < 'enum A' is always false}}<br>
+ if (0 < a)<br>
return 0;<br>
<br>
if (a < 0U)<br>
@@ -291,19 +296,19 @@ int main() {<br>
<br>
if (c < 0)<br>
return 0;<br>
- if (0 >= c) // expected-warning {{comparison 0 >= 'enum C' is always true}}<br>
+ if (0 >= c)<br>
return 0;<br>
- if (c > 0) // expected-warning {{comparison 'enum C' > 0 is always false}}<br>
+ if (c > 0)<br>
return 0;<br>
if (0 <= c)<br>
return 0;<br>
- if (c <= 0) // expected-warning {{comparison 'enum C' <= 0 is always true}}<br>
+ if (c <= 0)<br>
return 0;<br>
if (0 > c)<br>
return 0;<br>
if (c >= 0)<br>
return 0;<br>
- if (0 < c) // expected-warning {{0 < 'enum C' is always false}}<br>
+ if (0 < c)<br>
return 0;<br>
<br>
if (c < 0U)<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div>
<br>______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div>