[PATCH] D71576: [c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.

2019-12-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith marked 3 inline comments as done.
rsmith added inline comments.



Comment at: clang/lib/AST/Type.cpp:1865-1866
   // enumeration type in the sense required here.
   // C++0x: However, if the underlying type of the enum is fixed, it is
   // considered complete.
   if (const auto *ET = dyn_cast(CanonicalType))

rsmith wrote:
> rnk wrote:
> > Is this C++11 comment still relevant? I assume that `isComplete` handles 
> > this case by returning true, and a forward decl can tell us if the enum is 
> > scoped.
> I don't think this is useful -- and probably nor is the `isComplete` check. 
> I'll look into this in a follow-up commit.
Fixed in rGeea8ba097c4a86632b88291bea51eb710f8ae4fb. Turns out this was hiding 
a bug in how we checked `static_cast(...)` for incomplete enum types.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71576/new/

https://reviews.llvm.org/D71576



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71576: [c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.

2019-12-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith closed this revision.
rsmith marked 4 inline comments as done.
rsmith added a comment.

Committed as rG4b0029995853fe37d1dc95ef96f46697c743fcad 
.




Comment at: clang/lib/AST/Type.cpp:1865-1866
   // enumeration type in the sense required here.
   // C++0x: However, if the underlying type of the enum is fixed, it is
   // considered complete.
   if (const auto *ET = dyn_cast(CanonicalType))

rnk wrote:
> Is this C++11 comment still relevant? I assume that `isComplete` handles this 
> case by returning true, and a forward decl can tell us if the enum is scoped.
I don't think this is useful -- and probably nor is the `isComplete` check. 
I'll look into this in a follow-up commit.



Comment at: clang/test/SemaCXX/warn-enum-compare.cpp:79
 
-  while (B1 == B2); // expected-warning  {{comparison of two values with 
different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (name1::B2 == name2::B3); // expected-warning  {{comparison of two 
values with different enumeration types ('name1::Baz' and 'name2::Baz')}}

rnk wrote:
> It seems more technically correct to say that two values are being compared, 
> but I don't see how to keep the diagnostic as well factored as you have it.
Yeah, I agonized over this a little, but while I agree with you, in the end I 
think it's not entirely wrong to talk about (eg) a comparing an int with a 
float, and the ambiguity between types and values there doesn't seem likely to 
actually be confusing -- I don't think the extra words help comprehension of 
the diagnostic. I'm happy to change it back, of course.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71576/new/

https://reviews.llvm.org/D71576



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71576: [c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.

2019-12-16 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

I have no blocking concerns, just some idle thoughts. Up to you if you want 
Aaron's feedback before landing.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6235
+  "%plural{2:with|4:from|:and}0 "
+  "%select{enumeration|floating-point}1 type %3 is deprecated">,
+  InGroup;

I'm surprised we don't have a TableGen facility to say `Warning`, but I don't see an alternative.



Comment at: clang/lib/AST/Type.cpp:1865-1866
   // enumeration type in the sense required here.
   // C++0x: However, if the underlying type of the enum is fixed, it is
   // considered complete.
   if (const auto *ET = dyn_cast(CanonicalType))

Is this C++11 comment still relevant? I assume that `isComplete` handles this 
case by returning true, and a forward decl can tell us if the enum is scoped.



Comment at: clang/test/SemaCXX/warn-enum-compare.cpp:79
 
-  while (B1 == B2); // expected-warning  {{comparison of two values with 
different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (name1::B2 == name2::B3); // expected-warning  {{comparison of two 
values with different enumeration types ('name1::Baz' and 'name2::Baz')}}

It seems more technically correct to say that two values are being compared, 
but I don't see how to keep the diagnostic as well factored as you have it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71576/new/

https://reviews.llvm.org/D71576



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71576: [c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.

2019-12-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith created this revision.
rsmith added reviewers: aaron.ballman, rnk.
Herald added a project: clang.

This covers:

- usual arithmetic conversions (comparisons, arithmetic, conditionals) between 
different enumeration types
- usual arithmetic conversions between enums and floating-point types
- comparisons between two operands of array type

The deprecation warnings are on-by-default (in C++20 compilations); it
seems likely that these forms will become ill-formed in C++23, so
warning on them now by default seems wise.

For the first two bullets, off-by-default warnings were also added for
all the cases where we didn't already have warnings (covering language
modes prior to C++20). These warnings are in subgroups of the existing
-Wenum-conversion (except that the first case is not warned on if either
enumeration type is anonymous, consistent with our existing
-Wenum-conversion warnings).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71576

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CXX/expr/expr.arith.conv/p2.cpp
  clang/test/Sema/switch.c
  clang/test/Sema/warn-conditional-emum-types-mismatch.c
  clang/test/SemaCXX/deprecated.cpp
  clang/test/SemaCXX/self-comparison.cpp
  clang/test/SemaCXX/warn-enum-compare.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -926,18 +926,16 @@
 
   Consistent comparison (operator=)
   https://wg21.link/p0515r3;>P0515R3
-  SVN
+  SVN
 

 https://wg21.link/p0905r1;>P0905R1
   

 https://wg21.link/p1120r0;>P1120R0
-Partial
   

 https://wg21.link/p1185r2;>P1185R2
-SVN
   

 https://wg21.link/p1186r3;>P1186R3
Index: clang/test/SemaCXX/warn-enum-compare.cpp
===
--- clang/test/SemaCXX/warn-enum-compare.cpp
+++ clang/test/SemaCXX/warn-enum-compare.cpp
@@ -76,184 +76,184 @@
   while (td == AnonAA);  // expected-warning {{comparison of constant 'AnonAA' (42) with expression of type 'TD' is always false}}
 #endif
 
-  while (B1 == B2); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (name1::B2 == name2::B3); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (z == name2::B2); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-
-  while (B1 == B2); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (name1::B2 == (name2::B3)); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (z == name2::B2); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-
-  while B1))) == (((B2; // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while ((name1::B2) == (((name2::B3; // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while z))) == (name2::B2)); // expected-warning  {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}}
-
-  while (x == a); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'name1::Foo')}}
-  while (x == b); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'oneFoo' (aka 'name1::Foo'))}}
-  while (x == c); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'twoFoo' (aka 'name1::Foo'))}}
-
-  while (x == y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x != y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x >= y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x <= y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x > y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-  while (x < y); // expected-warning  {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
-
-  while (FooB == y); // expected-warning