[PATCH] D67247: Added missing unqualified name lookup of operator overloads for fold expressions

[Jonathan Meier via Phabricator via cfe-commits]

jonathanmeier updated this revision to Diff 225928.
jonathanmeier changed the repository for this revision from rC Clang to rG LLVM 
Github Monorepo.
jonathanmeier added a comment.

- Rebased to adapt to the latest changes for spaceship operator and comparison 
operator rewrite support in rL375305  and 
rL375306 .
- Added tests for comparison operator rewrites in fold expressions.
- Changed to using `llvm::iterator_range` instead of separate begin/end 
iterators.

ping @rsmith


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67247

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/UnresolvedSet.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/SemaTemplate/cxx1z-fold-expressions.cpp

Index: clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
===
--- clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
@@ -79,6 +79,36 @@
 static_assert((a, ::b, ::B::c, ::B::C::d, ::B::C::D::e) == );
 
 #if __cplusplus > 201703L
+
+namespace N {
+
+  struct Bool {
+constexpr Bool(const bool& b) : b(b) {}
+bool b;
+  };
+
+}
+
+constexpr bool operator==(const N::Bool& b1, const N::Bool& b2) { return b1.b == b2.b; }
+constexpr int operator<=>(const N::Bool& b1, const N::Bool& b2) { return b1.b - b2.b; }
+
+template constexpr auto fold_eq(T ...t) { return (t == ...); }
+template constexpr auto fold_neq(T ...t) { return (t != ...); }
+template constexpr auto fold_le(T ...t) { return (t < ...); }
+template constexpr auto fold_leq(T ...t) { return (t <= ...); }
+template constexpr auto fold_ge(T ...t) { return (t > ...); }
+template constexpr auto fold_geq(T ...t) { return (t >= ...); }
+
+static_assert(fold_eq(N::Bool{true}, N::Bool{true}, N::Bool{true}, N::Bool{true}, N::Bool{true}));
+static_assert(!fold_eq(N::Bool{true}, N::Bool{true}, N::Bool{false}, N::Bool{true}, N::Bool{true}));
+static_assert(fold_neq(N::Bool{true}, N::Bool{true}, N::Bool{true}, N::Bool{true}, N::Bool{true}));
+static_assert(!fold_neq(N::Bool{true}, N::Bool{true}, N::Bool{false}, N::Bool{true}, N::Bool{true}));
+
+static_assert(!fold_le(N::Bool{true}, N::Bool{true}, N::Bool{true}, N::Bool{true}, N::Bool{true}));
+static_assert(fold_leq(N::Bool{false}, N::Bool{true}, N::Bool{true}, N::Bool{true}, N::Bool{true}));
+static_assert(fold_ge(N::Bool{true}, N::Bool{false}, N::Bool{true}, N::Bool{false}, N::Bool{false}));
+static_assert(!fold_ge(N::Bool{false}, N::Bool{false}, N::Bool{true}, N::Bool{false}, N::Bool{false}));
+
 // The <=> operator is unique among binary operators in not being a
 // fold-operator.
 // FIXME: This diagnostic is not great.
@@ -102,3 +132,49 @@
 
   Sum<1>::type<1, 2> x; // expected-note {{instantiation of}}
 }
+
+namespace N {
+  
+  struct A { int i; };
+  struct B { int i; };
+  
+  constexpr B operator+(const B& a, const B& b) { return { a.i + b.i }; }
+  
+}
+
+struct C { int i; };
+
+constexpr C operator+(const C& a, const C& b) { return { a.i + b.i }; }
+constexpr N::A operator+(const N::A& a, const N::A& b) { return { a.i + b.i }; }
+
+template constexpr auto custom_fold(T1 t1, T2 ...t2) {
+  return (t2 + ...) + (... + t2) + (t2 + ... + t1) + (t1 + ... + t2);
+}
+
+static_assert(custom_fold(N::A{1}, N::A{2}, N::A{3}, N::A{4}, N::A{5}).i == 58);
+static_assert(custom_fold(N::B{1}, N::B{2}, N::B{3}, N::B{4}, N::B{5}).i == 58);
+static_assert(custom_fold(C{1}, C{2}, C{3}, C{4}, C{5}).i == 58);
+
+template constexpr auto func_fold(
+decltype((T{ I2 } + ...) + (... + T{ I2 }) + (T{ I2 } + ... + T{ I1 }) + (T{ I1 } + ... + T{ I2 })) t) {
+  return t.i;
+}
+
+static_assert(func_fold(N::A{ 42 }) == 42);
+static_assert(func_fold(N::B{ 42 }) == 42);
+static_assert(func_fold(C{ 42 }) == 42);
+
+struct D { int i; };
+
+namespace N {
+  
+  constexpr D operator+(const D& a, const D& b) { return { a.i + b.i }; }
+  
+}
+
+template constexpr auto custom_fold_using(T1 t1, T2 ...t2) {
+  using N::operator+;
+  return (t2 + ...) + (... + t2) + (t2 + ... + t1) + (t1 + ... + t2);
+}
+
+static_assert(custom_fold_using(D{1}, D{2}, D{3}, D{4}, D{5}).i == 58);
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1843,6 +1843,7 @@
 
 void ASTStmtWriter::VisitCXXFoldExpr(CXXFoldExpr *E) {
   VisitExpr(E);
+  Record.push_back(E->getNumOverloadCands());
   Record.AddSourceLocation(E->LParenLoc);
   Record.AddSourceLocation(E->EllipsisLoc);
   

[PATCH] D67247: Added missing unqualified name lookup of operator overloads for fold expressions

[Jonathan Meier via Phabricator via cfe-commits]

jonathanmeier added a comment.

ping @rsmith


Repository:
  rC Clang

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

https://reviews.llvm.org/D67247



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

[PATCH] D67247: Added missing unqualified name lookup of operator overloads for fold expressions

[Jonathan Meier via Phabricator via cfe-commits]

jonathanmeier created this revision.
jonathanmeier added reviewers: clang, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Valid operator overloads for fold expressions are not found in the current 
scope, since unqualified name lookup is not performed.

This is a proposal to fix Bug 30590 
 (and its duplicates Bug 30738 
 and Bug 42518 
). The core issue is that fold 
expressions get expanded during template instantiation (second phase of name 
lookup) when the current scope is not available anymore (current scope is 
available in the first phase of name lookup, i.e. while parsing the template 
definition).

My approach is to attach to the fold expression the operator overload 
candidates found by the unqualified name lookup while parsing. During template 
instantiation these candidates are then considered when the fold expressions 
are expanded and the actual binary operations are built.

Please comment on whether you deem this a reasonable approach or if you know of 
a better way of solving the issue.


Repository:
  rC Clang

https://reviews.llvm.org/D67247

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/UnresolvedSet.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/SemaTemplate/cxx1z-fold-expressions.cpp

Index: clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
===
--- clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
@@ -102,3 +102,49 @@
 
   Sum<1>::type<1, 2> x; // expected-note {{instantiation of}}
 }
+
+namespace N {
+  
+  struct A { int i; };
+  struct B { int i; };
+  
+  constexpr B operator+(const B& a, const B& b) { return { a.i + b.i }; }
+  
+}
+
+struct C { int i; };
+
+constexpr C operator+(const C& a, const C& b) { return { a.i + b.i }; }
+constexpr N::A operator+(const N::A& a, const N::A& b) { return { a.i + b.i }; }
+
+template constexpr auto custom_fold(T1 t1, T2 ...t2) {
+  return (t2 + ...) + (... + t2) + (t2 + ... + t1) + (t1 + ... + t2);
+}
+
+static_assert(custom_fold(N::A{1}, N::A{2}, N::A{3}, N::A{4}, N::A{5}).i == 58);
+static_assert(custom_fold(N::B{1}, N::B{2}, N::B{3}, N::B{4}, N::B{5}).i == 58);
+static_assert(custom_fold(C{1}, C{2}, C{3}, C{4}, C{5}).i == 58);
+
+template constexpr auto func_fold(
+decltype((T{ I2 } + ...) + (... + T{ I2 }) + (T{ I2 } + ... + T{ I1 }) + (T{ I1 } + ... + T{ I2 })) t) {
+  return t.i;
+}
+
+static_assert(func_fold(N::A{ 42 }) == 42);
+static_assert(func_fold(N::B{ 42 }) == 42);
+static_assert(func_fold(C{ 42 }) == 42);
+
+struct D { int i; };
+
+namespace N {
+  
+  constexpr D operator+(const D& a, const D& b) { return { a.i + b.i }; }
+  
+}
+
+template constexpr auto custom_fold_using(T1 t1, T2 ...t2) {
+  using N::operator+;
+  return (t2 + ...) + (... + t2) + (t2 + ... + t1) + (t1 + ... + t2);
+}
+
+static_assert(custom_fold_using(D{1}, D{2}, D{3}, D{4}, D{5}).i == 58);
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1817,6 +1817,7 @@
 
 void ASTStmtWriter::VisitCXXFoldExpr(CXXFoldExpr *E) {
   VisitExpr(E);
+  Record.push_back(E->getNumOverloadCands());
   Record.AddSourceLocation(E->LParenLoc);
   Record.AddSourceLocation(E->EllipsisLoc);
   Record.AddSourceLocation(E->RParenLoc);
@@ -1824,6 +1825,12 @@
   Record.AddStmt(E->SubExprs[0]);
   Record.AddStmt(E->SubExprs[1]);
   Record.push_back(E->Opcode);
+  for (UnresolvedSetIterator I = E->overloadCandsBegin(),
+ End = E->overloadCandsEnd();
+   I != End; ++I) {
+Record.AddDeclRef(I.getDecl());
+Record.push_back(I.getAccess());
+  }
   Code = serialization::EXPR_CXX_FOLD;
 }
 
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1883,6 +1883,7 @@
 
 void ASTStmtReader::VisitCXXFoldExpr(CXXFoldExpr *E) {
   VisitExpr(E);
+  unsigned NumOverloadCands = Record.readInt();
   E->LParenLoc = ReadSourceLocation();
   E->EllipsisLoc = ReadSourceLocation();
   E->RParenLoc = ReadSourceLocation();
@@ -1890,6 +1891,13 @@
   E->SubExprs[0] = Record.readSubExpr();
   E->SubExprs[1] = Record.readSubExpr();
   E->Opcode = (BinaryOperatorKind)Record.readInt();
+
+  DeclAccessPair *OverloadCands = E->getTrailingObjects();
+  for (unsigned I = 0; I != NumOverloadCands; ++I) {
+