[PATCH] D69276: Add missing assertions in testcase

2019-10-21 Thread 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.

Add two missing assertions for testcases introduced in rL373874 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69276

Files:
  clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp


Index: clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
===
--- clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
+++ clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
@@ -391,9 +391,11 @@
 };
 constexpr IdentityInStruct identity2a = {42};
 constexpr unsigned char identity2b = __builtin_bit_cast(unsigned char, 
identity2a.n);
+static_assert(identity2b == 42);
 
 union IdentityInUnion {
   unsigned char n;
 };
 constexpr IdentityInUnion identity3a = {42};
 constexpr unsigned char identity3b = __builtin_bit_cast(unsigned char, 
identity3a.n);
+static_assert(identity3b == 42);


Index: clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
===
--- clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
+++ clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
@@ -391,9 +391,11 @@
 };
 constexpr IdentityInStruct identity2a = {42};
 constexpr unsigned char identity2b = __builtin_bit_cast(unsigned char, identity2a.n);
+static_assert(identity2b == 42);
 
 union IdentityInUnion {
   unsigned char n;
 };
 constexpr IdentityInUnion identity3a = {42};
 constexpr unsigned char identity3b = __builtin_bit_cast(unsigned char, identity3a.n);
+static_assert(identity3b == 42);
___
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

2019-10-21 Thread 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] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

2019-10-11 Thread Jonathan Meier via Phabricator via cfe-commits 
jonathanmeier added a comment.

In D68682#1702025 , @poelmanc wrote:

> In D68682#1700908 , @Eugene.Zelenko 
> wrote:
>
> > You may be interested to also look on PR43583 related to 
> > readability-redundant-member-init.
>
>
> Thanks Eugene, I'm having trouble finding that. 
> https://reviews.llvm.org/D43583 seems related to MIPS instructions rather 
> than readability-redundant-member-init. Could you please post a link? Thanks.


PRs are bug reports. You can access them like this: https://llvm.org/PR43583


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682



___
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

2019-09-16 Thread 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] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-09-12 Thread Jonathan Meier via Phabricator via cfe-commits 
jonathanmeier added a comment.

Nice! I don't have commit access, so we'll need someone else have another look. 
@alexfh, since you previously worked on this, would you mind taking a look at 
this patch?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



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

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-09-12 Thread Jonathan Meier via Phabricator via cfe-commits 
jonathanmeier added a comment.

Thanks, this looks much better now. I think there's no need to track nesting of 
parenthesis, brackets and braces separately, so we can collapse `ParenLevel`, 
`BraceLevel` and `SquareBracketLevel` into a single `NestingLevel`, which 
simplifies all the conditions quite a bit.

Also consider adding a few more testcases, especially with nested templates and 
multiple template arguments (the reason for this patch, after all! ;-) ). You 
can easily test (different numbers of) multiple template arguments by just 
adding a few more template parameters with default arguments to e.g. 
`TwoArgTemplate`, `S` or `Q`.




Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:90
+  AngleBracketLevel == 0) {
+// If there is comma not nested then it is two or more declarations in 
this chain.
 return false;

change to `If there is a non-nested comma we have two or more declarations in 
this chain.`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



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

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-09-12 Thread Jonathan Meier via Phabricator via cfe-commits 
jonathanmeier added a comment.

Also note that your enhanced version of my first example actually works with 
your initial patch, since the two comparison operators cancel each other out in 
the counting logic.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



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

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-09-12 Thread Jonathan Meier via Phabricator via cfe-commits 
jonathanmeier added a comment.

Unfortunately, only considering parenthesis (`l_paren`, `r_paren`) is not 
sufficient. We need to consider all the nesting tokens, including brackets 
(`l_square`, `r_square`) and braces (`l_brace`, `r_brace`), since the Standard 
says (C++ 17 Draft N4659, Section 17.2/3 
):

> [...] When parsing a template-argument-list, the first non-nested > is taken 
> as the ending delimiter rather than a greater-than operator. [...]

Example with brackets that fails with your updated patch:

  template 
  struct S {};
  
  constexpr bool b[1] = { true };
  
  typedef S S_t, *S_p;

The following is an example with braces inside a template argument:

  struct T {
constexpr T(bool) {}

static constexpr bool b = true;  
  };
  
  typedef S S_t, *S_p;

Note though, that the current code aborts upon encountering braces to avoid 
removing a `typedef struct {...} T;` case and therefore, isn't even able to 
handle a single declaration chain with braces in a template argument, such as 
`typedef S S_t;`.

As to handling the comma cases:
(A) It is certainly not straightforward, since typedef declaration chains with 
multiple declarations are internally split up into separate declarations and 
the checker is called for each of these declarations individually.
(B) Your expansion would be valid, however, I think it might be easier to 
implement the expansion to

  using S_t = S<(0 < 0)>;
  using S_p = S<(0 < 0)>*;


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



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

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-09-11 Thread Jonathan Meier via Phabricator via cfe-commits 
jonathanmeier added a comment.

With non-type template parameters we can have expressions inside template 
arguments, including comparison operators like `<`, `<=`, `>` and `>=`, which 
lets us write typedefs like this:

  template 
  struct S {};
  
  typedef S<(0 < 0)> S_t, *S_p;

Unfortunately, for this example your patch breaks the check in the `tok::comma` 
case, which should abort the removal when there are multiple declarations in 
the declaration chain. It thinks the comma is still part of the template 
argument, since it expects a matching end template angle bracket for the less 
than operator which was erroneously interpreted as the start of a template 
argument.

With the `-fix` option, clang-tidy produces the following invalid using 
declaration for the example above:

  using S_t = S<(0 < 0)>, *S_p;


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



___
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

2019-09-05 Thread 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) {
+