[PATCH] D44988: [Sema] Fix decrement availability for built-in types

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329804: [Sema] Fix built-in decrement operator overload 
resolution (authored by jkorous, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44988?vs=141865=141995#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44988

Files:
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp


Index: cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp
===
--- cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp
+++ cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp
@@ -62,6 +62,10 @@
   // FIXME: should pass (void)static_cast(islong(e1 % e2));
 }
 
+struct BoolRef {
+  operator bool&();
+};
+
 struct ShortRef { // expected-note{{candidate function (the implicit copy 
assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment 
operator) not viable}}
@@ -73,6 +77,10 @@
   operator volatile long&();
 };
 
+struct FloatRef {
+  operator float&();
+};
+
 struct XpmfRef { // expected-note{{candidate function (the implicit copy 
assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment 
operator) not viable}}
@@ -84,13 +92,19 @@
   operator E2&();
 };
 
-void g(ShortRef sr, LongRef lr, E2Ref e2_ref, XpmfRef pmf_ref) {
+void g(BoolRef br, ShortRef sr, LongRef lr, FloatRef fr, E2Ref e2_ref, XpmfRef 
pmf_ref) {
   // C++ [over.built]p3
   short s1 = sr++;
 
-  // C++ [over.built]p3
+  // C++ [over.built]p4
   long l1 = lr--;
 
+  // C++ [over.built]p4
+  float f1 = fr--;
+
+  // C++ [over.built]p4
+  bool b2 = br--; // expected-error{{cannot decrement value of type 'BoolRef'}}
+
   // C++ [over.built]p18
   short& sr1 = (sr *= lr);
   volatile long& lr1 = (lr *= sr);
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -7796,10 +7796,12 @@
 if (!HasArithmeticOrEnumeralCandidateType)
   return;
 
-for (unsigned Arith = (Op == OO_PlusPlus? 0 : 1);
- Arith < NumArithmeticTypes; ++Arith) {
+for (unsigned Arith = 0; Arith < NumArithmeticTypes; ++Arith) {
+  const auto TypeOfT = ArithmeticTypes[Arith];
+  if (Op == OO_MinusMinus && TypeOfT == S.Context.BoolTy)
+continue;
   addPlusPlusMinusMinusStyleOverloads(
-ArithmeticTypes[Arith],
+TypeOfT,
 VisibleTypeConversionsQuals.hasVolatile(),
 VisibleTypeConversionsQuals.hasRestrict());
 }


Index: cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp
===
--- cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp
+++ cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp
@@ -62,6 +62,10 @@
   // FIXME: should pass (void)static_cast(islong(e1 % e2));
 }
 
+struct BoolRef {
+  operator bool&();
+};
+
 struct ShortRef { // expected-note{{candidate function (the implicit copy assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment operator) not viable}}
@@ -73,6 +77,10 @@
   operator volatile long&();
 };
 
+struct FloatRef {
+  operator float&();
+};
+
 struct XpmfRef { // expected-note{{candidate function (the implicit copy assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment operator) not viable}}
@@ -84,13 +92,19 @@
   operator E2&();
 };
 
-void g(ShortRef sr, LongRef lr, E2Ref e2_ref, XpmfRef pmf_ref) {
+void g(BoolRef br, ShortRef sr, LongRef lr, FloatRef fr, E2Ref e2_ref, XpmfRef pmf_ref) {
   // C++ [over.built]p3
   short s1 = sr++;
 
-  // C++ [over.built]p3
+  // C++ [over.built]p4
   long l1 = lr--;
 
+  // C++ [over.built]p4
+  float f1 = fr--;
+
+  // C++ [over.built]p4
+  bool b2 = br--; // expected-error{{cannot decrement value of type 'BoolRef'}}
+
   // C++ [over.built]p18
   short& sr1 = (sr *= lr);
   volatile long& lr1 = (lr *= sr);
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -7796,10 +7796,12 @@
 if (!HasArithmeticOrEnumeralCandidateType)
   return;
 
-for (unsigned Arith = (Op == OO_PlusPlus? 0 : 1);
- Arith < NumArithmeticTypes; ++Arith) {
+for (unsigned Arith = 0; Arith < NumArithmeticTypes; ++Arith) {
+  const auto TypeOfT = ArithmeticTypes[Arith];
+  if (Op == OO_MinusMinus && TypeOfT == S.Context.BoolTy)
+continue;
   

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai accepted this revision.
vsapsai added a comment.
This revision is now accepted and ready to land.

Looks good to me.


https://reviews.llvm.org/D44988



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

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added a comment.

Spot on with the negative test idea! Should've done that myself. Thanks.


https://reviews.llvm.org/D44988



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

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple updated this revision to Diff 141865.
jkorous-apple added a comment.

Added test for decrement being disabled for bool.
Fixed comment in test (will put into separate NFC commit).


https://reviews.llvm.org/D44988

Files:
  Sema/SemaOverload.cpp
  SemaCXX/overloaded-builtin-operators.cpp


Index: SemaCXX/overloaded-builtin-operators.cpp
===
--- SemaCXX/overloaded-builtin-operators.cpp
+++ SemaCXX/overloaded-builtin-operators.cpp
@@ -62,6 +62,10 @@
   // FIXME: should pass (void)static_cast(islong(e1 % e2));
 }
 
+struct BoolRef {
+  operator bool&();
+};
+
 struct ShortRef { // expected-note{{candidate function (the implicit copy 
assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment 
operator) not viable}}
@@ -73,6 +77,10 @@
   operator volatile long&();
 };
 
+struct FloatRef {
+  operator float&();
+};
+
 struct XpmfRef { // expected-note{{candidate function (the implicit copy 
assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment 
operator) not viable}}
@@ -84,13 +92,19 @@
   operator E2&();
 };
 
-void g(ShortRef sr, LongRef lr, E2Ref e2_ref, XpmfRef pmf_ref) {
+void g(BoolRef br, ShortRef sr, LongRef lr, FloatRef fr, E2Ref e2_ref, XpmfRef 
pmf_ref) {
   // C++ [over.built]p3
   short s1 = sr++;
 
-  // C++ [over.built]p3
+  // C++ [over.built]p4
   long l1 = lr--;
 
+  // C++ [over.built]p4
+  float f1 = fr--;
+
+  // C++ [over.built]p4
+  bool b2 = br--; // expected-error{{cannot decrement value of type 'BoolRef'}}
+
   // C++ [over.built]p18
   short& sr1 = (sr *= lr);
   volatile long& lr1 = (lr *= sr);
Index: Sema/SemaOverload.cpp
===
--- Sema/SemaOverload.cpp
+++ Sema/SemaOverload.cpp
@@ -7796,10 +7796,12 @@
 if (!HasArithmeticOrEnumeralCandidateType)
   return;
 
-for (unsigned Arith = (Op == OO_PlusPlus? 0 : 1);
- Arith < NumArithmeticTypes; ++Arith) {
+for (unsigned Arith = 0; Arith < NumArithmeticTypes; ++Arith) {
+  const auto TypeOfT = ArithmeticTypes[Arith];
+  if (Op == OO_MinusMinus && TypeOfT == S.Context.BoolTy)
+continue;
   addPlusPlusMinusMinusStyleOverloads(
-ArithmeticTypes[Arith],
+TypeOfT,
 VisibleTypeConversionsQuals.hasVolatile(),
 VisibleTypeConversionsQuals.hasRestrict());
 }


Index: SemaCXX/overloaded-builtin-operators.cpp
===
--- SemaCXX/overloaded-builtin-operators.cpp
+++ SemaCXX/overloaded-builtin-operators.cpp
@@ -62,6 +62,10 @@
   // FIXME: should pass (void)static_cast(islong(e1 % e2));
 }
 
+struct BoolRef {
+  operator bool&();
+};
+
 struct ShortRef { // expected-note{{candidate function (the implicit copy assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment operator) not viable}}
@@ -73,6 +77,10 @@
   operator volatile long&();
 };
 
+struct FloatRef {
+  operator float&();
+};
+
 struct XpmfRef { // expected-note{{candidate function (the implicit copy assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment operator) not viable}}
@@ -84,13 +92,19 @@
   operator E2&();
 };
 
-void g(ShortRef sr, LongRef lr, E2Ref e2_ref, XpmfRef pmf_ref) {
+void g(BoolRef br, ShortRef sr, LongRef lr, FloatRef fr, E2Ref e2_ref, XpmfRef pmf_ref) {
   // C++ [over.built]p3
   short s1 = sr++;
 
-  // C++ [over.built]p3
+  // C++ [over.built]p4
   long l1 = lr--;
 
+  // C++ [over.built]p4
+  float f1 = fr--;
+
+  // C++ [over.built]p4
+  bool b2 = br--; // expected-error{{cannot decrement value of type 'BoolRef'}}
+
   // C++ [over.built]p18
   short& sr1 = (sr *= lr);
   volatile long& lr1 = (lr *= sr);
Index: Sema/SemaOverload.cpp
===
--- Sema/SemaOverload.cpp
+++ Sema/SemaOverload.cpp
@@ -7796,10 +7796,12 @@
 if (!HasArithmeticOrEnumeralCandidateType)
   return;
 
-for (unsigned Arith = (Op == OO_PlusPlus? 0 : 1);
- Arith < NumArithmeticTypes; ++Arith) {
+for (unsigned Arith = 0; Arith < NumArithmeticTypes; ++Arith) {
+  const auto TypeOfT = ArithmeticTypes[Arith];
+  if (Op == OO_MinusMinus && TypeOfT == S.Context.BoolTy)
+continue;
   addPlusPlusMinusMinusStyleOverloads(
-ArithmeticTypes[Arith],
+TypeOfT,
 VisibleTypeConversionsQuals.hasVolatile(),
 VisibleTypeConversionsQuals.hasRestrict());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: test/SemaCXX/overloaded-builtin-operators.cpp:95-99
   // C++ [over.built]p3
   long l1 = lr--;
 
+  // C++ [over.built]p4
+  float f1 = fr--;

Looks like p3 for `lr--` is a typo because p3 is about `++` while p4 is about 
`--`. It means the existing test didn't catch this bug. Not sure that adding 
another positive test will reliably prevent a regression in the future. 
Currently `FloatRef` is the type to expose the bug because `FloatTy` is the 
first type in `ArithmeticTypes`.

What if we add a negative test? I.e. something that would fail like
```
cannot decrement value of type 'BoolRef'
```
Also we can improve testing for p3 and test that `BoolRef` does support 
increment.


Repository:
  rC Clang

https://reviews.llvm.org/D44988



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

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple created this revision.
jkorous-apple added reviewers: speziale.ettore, arphaman.
Herald added a subscriber: cfe-commits.

C++ [over.built]p4:

  For every pair (T, VQ), where T is an arithmetic type other than bool, and VQ 
is either volatile or empty, there exist candidate operator functions of the 
form
  
VQ T&  operator--(VQ T&);
T  operator--(VQ T&, int);

The bool type is in position LastPromotedIntegralType in 
BuiltinOperatorOverloadBuilder::getArithmeticType::ArithmeticTypes, but 
addPlusPlusMinusMinusArithmeticOverloads() is expecting it at position 0.

Original patch by Ettore Speziale.

rdar://problem/34255516


Repository:
  rC Clang

https://reviews.llvm.org/D44988

Files:
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/overloaded-builtin-operators.cpp


Index: test/SemaCXX/overloaded-builtin-operators.cpp
===
--- test/SemaCXX/overloaded-builtin-operators.cpp
+++ test/SemaCXX/overloaded-builtin-operators.cpp
@@ -73,6 +73,10 @@
   operator volatile long&();
 };
 
+struct FloatRef {
+  operator float&();
+};
+
 struct XpmfRef { // expected-note{{candidate function (the implicit copy 
assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment 
operator) not viable}}
@@ -84,13 +88,16 @@
   operator E2&();
 };
 
-void g(ShortRef sr, LongRef lr, E2Ref e2_ref, XpmfRef pmf_ref) {
+void g(ShortRef sr, LongRef lr, FloatRef fr, E2Ref e2_ref, XpmfRef pmf_ref) {
   // C++ [over.built]p3
   short s1 = sr++;
 
   // C++ [over.built]p3
   long l1 = lr--;
 
+  // C++ [over.built]p4
+  float f1 = fr--;
+
   // C++ [over.built]p18
   short& sr1 = (sr *= lr);
   volatile long& lr1 = (lr *= sr);
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -7796,10 +7796,12 @@
 if (!HasArithmeticOrEnumeralCandidateType)
   return;
 
-for (unsigned Arith = (Op == OO_PlusPlus? 0 : 1);
- Arith < NumArithmeticTypes; ++Arith) {
+for (unsigned Arith = 0; Arith < NumArithmeticTypes; ++Arith) {
+  const auto TypeOfT = ArithmeticTypes[Arith];
+  if (Op == OO_MinusMinus && TypeOfT == S.Context.BoolTy)
+continue;
   addPlusPlusMinusMinusStyleOverloads(
-ArithmeticTypes[Arith],
+TypeOfT,
 VisibleTypeConversionsQuals.hasVolatile(),
 VisibleTypeConversionsQuals.hasRestrict());
 }


Index: test/SemaCXX/overloaded-builtin-operators.cpp
===
--- test/SemaCXX/overloaded-builtin-operators.cpp
+++ test/SemaCXX/overloaded-builtin-operators.cpp
@@ -73,6 +73,10 @@
   operator volatile long&();
 };
 
+struct FloatRef {
+  operator float&();
+};
+
 struct XpmfRef { // expected-note{{candidate function (the implicit copy assignment operator) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate function (the implicit move assignment operator) not viable}}
@@ -84,13 +88,16 @@
   operator E2&();
 };
 
-void g(ShortRef sr, LongRef lr, E2Ref e2_ref, XpmfRef pmf_ref) {
+void g(ShortRef sr, LongRef lr, FloatRef fr, E2Ref e2_ref, XpmfRef pmf_ref) {
   // C++ [over.built]p3
   short s1 = sr++;
 
   // C++ [over.built]p3
   long l1 = lr--;
 
+  // C++ [over.built]p4
+  float f1 = fr--;
+
   // C++ [over.built]p18
   short& sr1 = (sr *= lr);
   volatile long& lr1 = (lr *= sr);
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -7796,10 +7796,12 @@
 if (!HasArithmeticOrEnumeralCandidateType)
   return;
 
-for (unsigned Arith = (Op == OO_PlusPlus? 0 : 1);
- Arith < NumArithmeticTypes; ++Arith) {
+for (unsigned Arith = 0; Arith < NumArithmeticTypes; ++Arith) {
+  const auto TypeOfT = ArithmeticTypes[Arith];
+  if (Op == OO_MinusMinus && TypeOfT == S.Context.BoolTy)
+continue;
   addPlusPlusMinusMinusStyleOverloads(
-ArithmeticTypes[Arith],
+TypeOfT,
 VisibleTypeConversionsQuals.hasVolatile(),
 VisibleTypeConversionsQuals.hasRestrict());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits