from:"Barry Revzin via Phabricator via cfe\-commits"

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-15 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added inline comments.



Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

royjacobson wrote:
> BRevzin wrote:
> > It's possible that I just don't understand what these tests actually mean 
> > but... where is the test for `DefaultConstructorCheck<2>` having a deleted 
> > default constructor, or `DefaultConstructorCheck<3>` having a non-defaulted 
> > one?
> > 
> > It'd also be worthwhile to have at least one test with constaints that 
> > subsume each other instead of being mutually exclusive. 
> Should I check this? Except destructors, the other SMFs are found during 
> overload resolution using the usual lookup that already takes into account 
> delete/default/constraints etc.
> 
> This patch is about making sure that we set the triviality attributes 
> correctly according to the eligible functions, so this is what I added tests 
> for.
> 
> Most of this testing is done in the sema test, but I need this AST test as 
> well to make sure we get the `canPassInRegisters` attribute correctly - we're 
> doing some custom processing over the functions without the usual type 
> attributes because there are some weird compatibility edge cases.
> 
One of the motivations for the paper is to ensure that like given:

```
template 
struct optional {
optional(optional const&) requires copyable && trivially_copyableT> = 
default;
optional(optional const&) requires copyable;
};
```

`optional` is copyable (but not trivially copyable), `optional` is 
trivially copyable, and `optional>` isn't copyable. I'm not 
sure what in here checks if that works. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-15 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added inline comments.



Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

It's possible that I just don't understand what these tests actually mean 
but... where is the test for `DefaultConstructorCheck<2>` having a deleted 
default constructor, or `DefaultConstructorCheck<3>` having a non-defaulted one?

It'd also be worthwhile to have at least one test with constaints that subsume 
each other instead of being mutually exclusive. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D78938: Make LLVM build in C++20 mode

2020-12-15 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added a comment.

In D78938#2450915 , @nlopes wrote:

> Thanks @lebedev.ri for the pointer!
> I started working on exactly the same thing as I was trying to link a C++20 
> project with LLVM.
> @BRevzin is there anything missing in this patch? Do you have commit access 
> or do you need help to land this?

There are two comments that @dblaikie made that need to still be addressed (one 
about renaming parameters of a function from `Self` and `Other` to `LHS` and 
`RHS`, and one about changing the ADL-inhibition strategy from 
`(to_address)(x)` to `llvm::to_address(x)`), both of which combined should take 
about a minute to do and then however long to compile.

I've forgotten how to push changes and am kind of confused at the state of my 
current branch anyway, and it takes so long to do anything on my laptop that 
I'm more than happy to let you take over. I do not have commit access anyway.

Per @MaskRay, I'm Barry Revzin .

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D78938: Make LLVM build in C++20 mode

2020-09-29 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added inline comments.



Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h:167-171
-
-inline bool operator!=(const DWARFExpression::iterator ,
-   const DWARFExpression::iterator ) {
-  return !(LHS == RHS);
-}

lebedev.ri wrote:
> BRevzin wrote:
> > dblaikie wrote:
> > > Why are some being removed? That seems harder to justify. Even if they're 
> > > not called, it may be more valuable to have the symmetry to reduce 
> > > friction if/when they are needed. (iterators seem pretty common to 
> > > compare for inequality - such as in a loop condition testing I != E)
> > They're not being removed. These functions still exist - it's just that now 
> > they're being injected by the base class template with this exact signature 
> > (rather than before where they were slightly different), so that now these 
> > are redefinition issues. 
> > 
> > There's no loss of functionality here. 
> Does LLVM still build fine in C++14/C++17 modes afterwards?
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D78938: Make LLVM build in C++20 mode

2020-09-29 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added inline comments.



Comment at: llvm/include/llvm/IR/BasicBlock.h:324-325
+template ::value>>
 phi_iterator_impl(const phi_iterator_impl )

dblaikie wrote:
> BRevzin wrote:
> > dblaikie wrote:
> > > What tripped over/required this SFINAE?
> > There's somewhere which compared a const iterator to a non-const iterator, 
> > that ends up doing conversions in both directions under C++20 rules, one 
> > direction of which is perfectly fine and the other was a hard error. Need 
> > to make the non-const iterator not constructible from a const iterator.
> Is this true for all iterators? Or some quirk of how this one is written/used 
> (that could be fixed/changed there instead)?
So I undid this change to copy the exact issue that I ran into. But it actually 
ended up still compiling anyway. Part of the issue might be that I keep futzing 
with the cmake configuration since it takes me more than an hour to compile, so 
maybe there's some target that needed this change that I no longer compile.

But the kind of problem I think this had was:

```
template 
struct iterator {
T* p;

template 
iterator(iterator rhs)
: p(rhs.p)
{ } 

bool operator==(iterator const& rhs);
};

bool check(iterator a, iterator b) {
return a == b;
}
```

which compiles fine in C++17 but is ambiguous in C++20 because 
`b.operator==(a)` is also a candidate (even though it's not _really_ a 
candidate, and would be a hard error if selected). the sfinae removes the bad 
candidate from the set. 

It's true for all iterators in general in that you want `const_iterator` 
constructible from `iterator` but not the reverse (unless they're the same 
type).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D78938: Make LLVM build in C++20 mode

2020-09-27 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added inline comments.



Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h:167-171
-
-inline bool operator!=(const DWARFExpression::iterator ,
-   const DWARFExpression::iterator ) {
-  return !(LHS == RHS);
-}

dblaikie wrote:
> Why are some being removed? That seems harder to justify. Even if they're not 
> called, it may be more valuable to have the symmetry to reduce friction 
> if/when they are needed. (iterators seem pretty common to compare for 
> inequality - such as in a loop condition testing I != E)
They're not being removed. These functions still exist - it's just that now 
they're being injected by the base class template with this exact signature 
(rather than before where they were slightly different), so that now these are 
redefinition issues. 

There's no loss of functionality here. 



Comment at: llvm/include/llvm/IR/BasicBlock.h:324-325
+template ::value>>
 phi_iterator_impl(const phi_iterator_impl )

dblaikie wrote:
> What tripped over/required this SFINAE?
There's somewhere which compared a const iterator to a non-const iterator, that 
ends up doing conversions in both directions under C++20 rules, one direction 
of which is perfectly fine and the other was a hard error. Need to make the 
non-const iterator not constructible from a const iterator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D78938: Make LLVM build in C++20 mode

2020-09-09 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added a comment.

> @BRevzin, you should a) mention the u8/const char* issue in the description 
> too, and also what compiler you used to build this with. I fully expect at 
> this stage that there are some C++20 compilers that might have slightly 
> different interpretations of things which this won't resolve, so knowing 
> which one this is intended to work with could help with historical research.

It's mentioned in the description. I built with clang-10.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-09-07 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added inline comments.



Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:805-817
+  return IsASCII ? "^" : (const char *)u8"\u2548";
 case LineChar::RangeMid:
-  return IsASCII ? "|" : u8"\u2503";
+  return IsASCII ? "|" : (const char *)u8"\u2503";
 case LineChar::RangeEnd:
-  return IsASCII ? "v" : u8"\u253b";
+  return IsASCII ? "v" : (const char *)u8"\u253b";
 case LineChar::LabelVert:
+  return IsASCII ? "|" : (const char *)u8"\u2502";

jhenderson wrote:
> This seems unrelated to comparison checking?
> This seems unrelated to comparison checking?

It is unrelated. But In C++20, `u8` literals become their own type so this no 
longer compiled and I wanted to ensure that I could actually run the tests. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-09-07 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added a comment.

In D78938#2258557 , @jhenderson wrote:

> Not that I have anything particularly against this, but won't this likely rot 
> fairly rapidly? It's not like LLVM is even on C++17 let alone C++20 yet, so 
> trying to make it work like the latter when it's just going to break again 
> seems a bit like wasted effort to me.

People will want to write C++20 programs that use LLVM headers, so I think it's 
important to help let them do that. Sure, it may rot, but incremental fixes 
down the line will be smaller.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-09-04 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin updated this revision to Diff 290023.
BRevzin added a comment.
Herald added subscribers: rupprecht, MaskRay.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.

Updating this review with some additional changes that need to be made since I 
last touched it, and some of the previous changes had inadvertently broken the 
C++14 build so fixing those as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

Files:
  clang/include/clang/AST/StmtIterator.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  llvm/include/llvm/ADT/AllocatorList.h
  llvm/include/llvm/ADT/DenseMap.h
  llvm/include/llvm/ADT/DenseSet.h
  llvm/include/llvm/ADT/DirectedGraph.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/StringMap.h
  llvm/include/llvm/ADT/iterator.h
  llvm/include/llvm/CodeGen/DIE.h
  llvm/include/llvm/CodeGen/LiveInterval.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/BasicBlock.h
  llvm/include/llvm/Object/StackMapParser.h
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/Support/BinaryStreamRef.h
  llvm/include/llvm/Support/SuffixTree.h
  llvm/lib/CodeGen/PeepholeOptimizer.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/ObjectYAML/DWARFEmitter.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/unittests/ADT/STLExtrasTest.cpp

Index: llvm/unittests/ADT/STLExtrasTest.cpp
===
--- llvm/unittests/ADT/STLExtrasTest.cpp
+++ llvm/unittests/ADT/STLExtrasTest.cpp
@@ -462,22 +462,23 @@
   EXPECT_EQ(V1, to_address(V1));
 
   // Check fancy pointer overload for unique_ptr
+  // Parenthesizing to_address to avoid ADL finding std::to_address
   std::unique_ptr V2 = std::make_unique(0);
-  EXPECT_EQ(V2.get(), to_address(V2));
+  EXPECT_EQ(V2.get(), (to_address)(V2));
 
   V2.reset(V1);
-  EXPECT_EQ(V1, to_address(V2));
+  EXPECT_EQ(V1, (to_address)(V2));
   V2.release();
 
   // Check fancy pointer overload for shared_ptr
   std::shared_ptr V3 = std::make_shared(0);
   std::shared_ptr V4 = V3;
   EXPECT_EQ(V3.get(), V4.get());
-  EXPECT_EQ(V3.get(), to_address(V3));
-  EXPECT_EQ(V4.get(), to_address(V4));
+  EXPECT_EQ(V3.get(), (to_address)(V3));
+  EXPECT_EQ(V4.get(), (to_address)(V4));
 
   V3.reset(V1);
-  EXPECT_EQ(V1, to_address(V3));
+  EXPECT_EQ(V1, (to_address)(V3));
 }
 
 TEST(STLExtrasTest, partition_point) {
Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -802,19 +802,19 @@
 bool IsASCII = DbgVariables == DVASCII;
 switch (C) {
 case LineChar::RangeStart:
-  return IsASCII ? "^" : u8"\u2548";
+  return IsASCII ? "^" : (const char *)u8"\u2548";
 case LineChar::RangeMid:
-  return IsASCII ? "|" : u8"\u2503";
+  return IsASCII ? "|" : (const char *)u8"\u2503";
 case LineChar::RangeEnd:
-  return IsASCII ? "v" : u8"\u253b";
+  return IsASCII ? "v" : (const char *)u8"\u253b";
 case LineChar::LabelVert:
-  return IsASCII ? "|" : u8"\u2502";
+  return IsASCII ? "|" : (const char *)u8"\u2502";
 case LineChar::LabelCornerNew:
-  return IsASCII ? "/" : u8"\u250c";
+  return IsASCII ? "/" : (const char *)u8"\u250c";
 case LineChar::LabelCornerActive:
-  return IsASCII ? "|" : u8"\u2520";
+  return IsASCII ? "|" : (const char *)u8"\u2520";
 case LineChar::LabelHoriz:
-  return IsASCII ? "-" : u8"\u2500";
+  return IsASCII ? "-" : (const char *)u8"\u2500";
 }
 llvm_unreachable("Unhandled LineChar enum");
   }
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -149,8 +149,8 @@
   // The instruction (VN) which uses the values flowing out of CHI.
   Instruction *I;
 
-  bool operator==(const CHIArg ) { return VN == A.VN; }
-  bool operator!=(const CHIArg ) { return !(*this == A); }
+  bool operator==(const CHIArg ) const { return VN == A.VN; }
+  bool operator!=(const CHIArg ) const { return !(*this == A); }
 };
 
 using CHIIt = SmallVectorImpl::iterator;
Index: llvm/lib/ObjectYAML/DWARFEmitter.cpp
===
--- llvm/lib/ObjectYAML/DWARFEmitter.cpp
+++ llvm/lib/ObjectYAML/DWARFEmitter.cpp
@@ -615,7 +615,7 @@
 writeInteger((uint8_t)TableEntry.SegSelectorSize, OS, DI.IsLittleEndian);
 
 for (const SegAddrPair  : TableEntry.SegAddrPairs) {
-  if (TableEntry.SegSelectorSize != 0)
+  if

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-05-25 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added inline comments.



Comment at: llvm/include/llvm/ADT/DirectedGraph.h:97
+  }
+  friend bool operator!=(const NodeType , const NodeType ) { !(M == N); }
 

jfb wrote:
> davidstone wrote:
> > Missing `return`
> 
> 
> Did this not trigger a diagnostic when building? I wonder if it's just not on?
Yeah I was surprised too. I'm compiling with `-Wall -Wextra`... 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938



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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-05-25 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin updated this revision to Diff 266071.
BRevzin marked 2 inline comments as done.
BRevzin added a comment.

- Explaining the cryptic parentheses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

Files:
  clang/include/clang/AST/StmtIterator.h
  clang/lib/Parse/ParseOpenMP.cpp
  llvm/include/llvm/ADT/AllocatorList.h
  llvm/include/llvm/ADT/DenseMap.h
  llvm/include/llvm/ADT/DenseSet.h
  llvm/include/llvm/ADT/DirectedGraph.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/StringMap.h
  llvm/include/llvm/ADT/iterator.h
  llvm/include/llvm/CodeGen/LiveInterval.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/BasicBlock.h
  llvm/include/llvm/Object/StackMapParser.h
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/Support/BinaryStreamRef.h
  llvm/lib/CodeGen/MachineOutliner.cpp
  llvm/lib/CodeGen/PeepholeOptimizer.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/unittests/ADT/STLExtrasTest.cpp

Index: llvm/unittests/ADT/STLExtrasTest.cpp
===
--- llvm/unittests/ADT/STLExtrasTest.cpp
+++ llvm/unittests/ADT/STLExtrasTest.cpp
@@ -462,22 +462,23 @@
   EXPECT_EQ(V1, to_address(V1));
 
   // Check fancy pointer overload for unique_ptr
+  // Parenthesizing to_address to avoid ADL finding std::to_address
   std::unique_ptr V2 = std::make_unique(0);
-  EXPECT_EQ(V2.get(), to_address(V2));
+  EXPECT_EQ(V2.get(), (to_address)(V2));
 
   V2.reset(V1);
-  EXPECT_EQ(V1, to_address(V2));
+  EXPECT_EQ(V1, (to_address)(V2));
   V2.release();
 
   // Check fancy pointer overload for shared_ptr
   std::shared_ptr V3 = std::make_shared(0);
   std::shared_ptr V4 = V3;
   EXPECT_EQ(V3.get(), V4.get());
-  EXPECT_EQ(V3.get(), to_address(V3));
-  EXPECT_EQ(V4.get(), to_address(V4));
+  EXPECT_EQ(V3.get(), (to_address)(V3));
+  EXPECT_EQ(V4.get(), (to_address)(V4));
 
   V3.reset(V1);
-  EXPECT_EQ(V1, to_address(V3));
+  EXPECT_EQ(V1, (to_address)(V3));
 }
 
 TEST(STLExtrasTest, partition_point) {
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -149,8 +149,8 @@
   // The instruction (VN) which uses the values flowing out of CHI.
   Instruction *I;
 
-  bool operator==(const CHIArg ) { return VN == A.VN; }
-  bool operator!=(const CHIArg ) { return !(*this == A); }
+  bool operator==(const CHIArg ) const { return VN == A.VN; }
+  bool operator!=(const CHIArg ) const { return !(*this == A); }
 };
 
 using CHIIt = SmallVectorImpl::iterator;
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -665,7 +665,7 @@
   if (auto *T = dyn_cast_or_null(Attachment.second))
 for (unsigned N = 0; N < T->getNumOperands(); ++N)
   if (auto *Loc = dyn_cast_or_null(T->getOperand(N)))
-if (Loc != DebugLoc())
+if (Loc != DebugLoc().get())
   T->replaceOperandWith(N, remapDebugLoc(Loc));
   }
 }
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -1729,7 +1729,7 @@
   return Alignment != 0;
 }
 
-bool AttrBuilder::operator==(const AttrBuilder ) {
+bool AttrBuilder::operator==(const AttrBuilder ) const {
   if (Attrs != B.Attrs)
 return false;
 
Index: llvm/lib/CodeGen/PeepholeOptimizer.cpp
===
--- llvm/lib/CodeGen/PeepholeOptimizer.cpp
+++ llvm/lib/CodeGen/PeepholeOptimizer.cpp
@@ -330,7 +330,7 @@
   return RegSrcs[Idx].SubReg;
 }
 
-bool operator==(const ValueTrackerResult ) {
+bool operator==(const ValueTrackerResult ) const {
   if (Other.getInst() != getInst())
 return false;
 
Index: llvm/lib/CodeGen/MachineOutliner.cpp
===
--- llvm/lib/CodeGen/MachineOutliner.cpp
+++ llvm/lib/CodeGen/MachineOutliner.cpp
@@ -590,10 +590,10 @@
   return It;
 }
 
-bool operator==(const RepeatedSubstringIterator ) {
+bool operator==(const RepeatedSubstringIterator ) const {
   return N == Other.N;
 }
-bool operator!=(const RepeatedSubstringIterator ) {
+bool operator!=(const RepeatedSubstringIterator ) const {
   return !(*this == Other);
 }
 
Index: llvm/include/llvm/Support/BinaryStreamRef.h
===
--- llvm/include/llvm/Support/BinaryStreamRef.h
+++ llvm/include/llvm/Support/BinaryStreamRef.h
@@ -121,12 +121,12 @@
 
   bool

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-05-25 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin updated this revision to Diff 265900.
BRevzin added a comment.

- Backing out changes that aren't strictly comparison-related.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

Files:
  clang/include/clang/AST/StmtIterator.h
  clang/lib/Parse/ParseOpenMP.cpp
  llvm/include/llvm/ADT/AllocatorList.h
  llvm/include/llvm/ADT/DenseMap.h
  llvm/include/llvm/ADT/DenseSet.h
  llvm/include/llvm/ADT/DirectedGraph.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/StringMap.h
  llvm/include/llvm/ADT/iterator.h
  llvm/include/llvm/CodeGen/LiveInterval.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/BasicBlock.h
  llvm/include/llvm/Object/StackMapParser.h
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/Support/BinaryStreamRef.h
  llvm/lib/CodeGen/MachineOutliner.cpp
  llvm/lib/CodeGen/PeepholeOptimizer.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/unittests/ADT/STLExtrasTest.cpp

Index: llvm/unittests/ADT/STLExtrasTest.cpp
===
--- llvm/unittests/ADT/STLExtrasTest.cpp
+++ llvm/unittests/ADT/STLExtrasTest.cpp
@@ -463,21 +463,21 @@
 
   // Check fancy pointer overload for unique_ptr
   std::unique_ptr V2 = std::make_unique(0);
-  EXPECT_EQ(V2.get(), to_address(V2));
+  EXPECT_EQ(V2.get(), (to_address)(V2));
 
   V2.reset(V1);
-  EXPECT_EQ(V1, to_address(V2));
+  EXPECT_EQ(V1, (to_address)(V2));
   V2.release();
 
   // Check fancy pointer overload for shared_ptr
   std::shared_ptr V3 = std::make_shared(0);
   std::shared_ptr V4 = V3;
   EXPECT_EQ(V3.get(), V4.get());
-  EXPECT_EQ(V3.get(), to_address(V3));
-  EXPECT_EQ(V4.get(), to_address(V4));
+  EXPECT_EQ(V3.get(), (to_address)(V3));
+  EXPECT_EQ(V4.get(), (to_address)(V4));
 
   V3.reset(V1);
-  EXPECT_EQ(V1, to_address(V3));
+  EXPECT_EQ(V1, (to_address)(V3));
 }
 
 TEST(STLExtrasTest, partition_point) {
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -149,8 +149,8 @@
   // The instruction (VN) which uses the values flowing out of CHI.
   Instruction *I;
 
-  bool operator==(const CHIArg ) { return VN == A.VN; }
-  bool operator!=(const CHIArg ) { return !(*this == A); }
+  bool operator==(const CHIArg ) const { return VN == A.VN; }
+  bool operator!=(const CHIArg ) const { return !(*this == A); }
 };
 
 using CHIIt = SmallVectorImpl::iterator;
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -665,7 +665,7 @@
   if (auto *T = dyn_cast_or_null(Attachment.second))
 for (unsigned N = 0; N < T->getNumOperands(); ++N)
   if (auto *Loc = dyn_cast_or_null(T->getOperand(N)))
-if (Loc != DebugLoc())
+if (Loc != DebugLoc().get())
   T->replaceOperandWith(N, remapDebugLoc(Loc));
   }
 }
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -1729,7 +1729,7 @@
   return Alignment != 0;
 }
 
-bool AttrBuilder::operator==(const AttrBuilder ) {
+bool AttrBuilder::operator==(const AttrBuilder ) const {
   if (Attrs != B.Attrs)
 return false;
 
Index: llvm/lib/CodeGen/PeepholeOptimizer.cpp
===
--- llvm/lib/CodeGen/PeepholeOptimizer.cpp
+++ llvm/lib/CodeGen/PeepholeOptimizer.cpp
@@ -330,7 +330,7 @@
   return RegSrcs[Idx].SubReg;
 }
 
-bool operator==(const ValueTrackerResult ) {
+bool operator==(const ValueTrackerResult ) const {
   if (Other.getInst() != getInst())
 return false;
 
Index: llvm/lib/CodeGen/MachineOutliner.cpp
===
--- llvm/lib/CodeGen/MachineOutliner.cpp
+++ llvm/lib/CodeGen/MachineOutliner.cpp
@@ -590,10 +590,10 @@
   return It;
 }
 
-bool operator==(const RepeatedSubstringIterator ) {
+bool operator==(const RepeatedSubstringIterator ) const {
   return N == Other.N;
 }
-bool operator!=(const RepeatedSubstringIterator ) {
+bool operator!=(const RepeatedSubstringIterator ) const {
   return !(*this == Other);
 }
 
Index: llvm/include/llvm/Support/BinaryStreamRef.h
===
--- llvm/include/llvm/Support/BinaryStreamRef.h
+++ llvm/include/llvm/Support/BinaryStreamRef.h
@@ -121,12 +121,12 @@
 
   bool valid() const { return BorrowedImpl != nullptr; }
 
-  bool operator==(const RefType ) const {
-if (BorrowedImpl !=

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-05-25 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin updated this revision to Diff 265875.
BRevzin added a comment.

- Adding missing return.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

Files:
  clang/include/clang/AST/StmtIterator.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  llvm/include/llvm/ADT/AllocatorList.h
  llvm/include/llvm/ADT/DenseMap.h
  llvm/include/llvm/ADT/DenseSet.h
  llvm/include/llvm/ADT/DirectedGraph.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/StringMap.h
  llvm/include/llvm/ADT/iterator.h
  llvm/include/llvm/CodeGen/LiveInterval.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/BasicBlock.h
  llvm/include/llvm/Object/StackMapParser.h
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/Support/BinaryStreamRef.h
  llvm/lib/CodeGen/MachineOutliner.cpp
  llvm/lib/CodeGen/PeepholeOptimizer.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/unittests/ADT/STLExtrasTest.cpp
  llvm/unittests/Support/DJBTest.cpp
  llvm/unittests/Support/JSONTest.cpp

Index: llvm/unittests/Support/JSONTest.cpp
===
--- llvm/unittests/Support/JSONTest.cpp
+++ llvm/unittests/Support/JSONTest.cpp
@@ -173,12 +173,14 @@
   Compare(R"("\"\\\b\f\n\r\t")", "\"\\\b\f\n\r\t");
   Compare(R"("\u")", llvm::StringRef("\0", 1));
   Compare("\"\x7f\"", "\x7f");
+#ifndef __cpp_char8_t
   Compare(R"("\ud801\udc37")", u8"\U00010437"); // UTF16 surrogate pair escape.
   Compare("\"\xE2\x82\xAC\xF0\x9D\x84\x9E\"", u8"\u20ac\U0001d11e"); // UTF8
   Compare(
   R"("LoneLeading=\ud801, LoneTrailing=\udc01, LeadingLeadingTrailing=\ud801\ud801\udc37")",
   u8"LoneLeading=\ufffd, LoneTrailing=\ufffd, "
   u8"LeadingLeadingTrailing=\ufffd\U00010437"); // Invalid unicode.
+#endif
 
   Compare(R"({"":0,"":0})", Object{{"", 0}});
   Compare(R"({"obj":{},"arr":[]})", Object{{"obj", Object{}}, {"arr", {}}});
Index: llvm/unittests/Support/DJBTest.cpp
===
--- llvm/unittests/Support/DJBTest.cpp
+++ llvm/unittests/Support/DJBTest.cpp
@@ -24,6 +24,7 @@
   {{""}, {""}},
 
   {{"I"}, {"i"}},
+#ifndef __cpp_char8_t
   // Latin Small Letter Dotless I
   {{u8"\u0130"}, {"i"}},
   // Latin Capital Letter I With Dot Above
@@ -47,6 +48,7 @@
   {{u8"\uff2d"}, {u8"\uff4d"}},
   // Old Hungarian Capital Letter Ej
   {{u8"\U00010c92"}, {u8"\U00010cd2"}},
+#endif
   };
 
   for (const TestCase  : Tests) {
@@ -79,6 +81,7 @@
   }
 }
 
+#ifndef __cpp_char8_t
 TEST(DJBTest, knownValuesUnicode) {
   EXPECT_EQ(5866553u, djbHash(u8"\u0130"));
   EXPECT_EQ(177678u, caseFoldingDjbHash(u8"\u0130"));
@@ -93,3 +96,4 @@
   u8"\u0130\u0131\u00c0\u00e0\u0100\u0101\u0139\u013a\u0415\u0435\u1ea6"
   u8"\u1ea7\u212a\u006b\u2c1d\u2c4d\uff2d\uff4d\U00010c92\U00010cd2"));
 }
+#endif
\ No newline at end of file
Index: llvm/unittests/ADT/STLExtrasTest.cpp
===
--- llvm/unittests/ADT/STLExtrasTest.cpp
+++ llvm/unittests/ADT/STLExtrasTest.cpp
@@ -463,21 +463,21 @@
 
   // Check fancy pointer overload for unique_ptr
   std::unique_ptr V2 = std::make_unique(0);
-  EXPECT_EQ(V2.get(), to_address(V2));
+  EXPECT_EQ(V2.get(), (to_address)(V2));
 
   V2.reset(V1);
-  EXPECT_EQ(V1, to_address(V2));
+  EXPECT_EQ(V1, (to_address)(V2));
   V2.release();
 
   // Check fancy pointer overload for shared_ptr
   std::shared_ptr V3 = std::make_shared(0);
   std::shared_ptr V4 = V3;
   EXPECT_EQ(V3.get(), V4.get());
-  EXPECT_EQ(V3.get(), to_address(V3));
-  EXPECT_EQ(V4.get(), to_address(V4));
+  EXPECT_EQ(V3.get(), (to_address)(V3));
+  EXPECT_EQ(V4.get(), (to_address)(V4));
 
   V3.reset(V1);
-  EXPECT_EQ(V1, to_address(V3));
+  EXPECT_EQ(V1, (to_address)(V3));
 }
 
 TEST(STLExtrasTest, partition_point) {
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -149,8 +149,8 @@
   // The instruction (VN) which uses the values flowing out of CHI.
   Instruction *I;
 
-  bool operator==(const CHIArg ) { return VN == A.VN; }
-  bool operator!=(const CHIArg ) { return !(*this == A); }
+  bool operator==(const CHIArg ) const { return VN == A.VN; }
+  bool operator!=(const CHIArg ) const { return !(*this == A); }
 };
 
 using CHIIt = SmallVectorImpl::iterator;
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -665,7 +665,7 @@
   if (auto *T = dyn_cast_or_null(Attachment.second))

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-05-25 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin updated this revision to Diff 265874.
BRevzin added a comment.
Herald added a subscriber: martong.

- A few more changes from tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

Files:
  clang/include/clang/AST/StmtIterator.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  llvm/include/llvm/ADT/AllocatorList.h
  llvm/include/llvm/ADT/DenseMap.h
  llvm/include/llvm/ADT/DenseSet.h
  llvm/include/llvm/ADT/DirectedGraph.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/StringMap.h
  llvm/include/llvm/ADT/iterator.h
  llvm/include/llvm/CodeGen/LiveInterval.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/BasicBlock.h
  llvm/include/llvm/Object/StackMapParser.h
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/Support/BinaryStreamRef.h
  llvm/lib/CodeGen/MachineOutliner.cpp
  llvm/lib/CodeGen/PeepholeOptimizer.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/unittests/ADT/STLExtrasTest.cpp
  llvm/unittests/Support/DJBTest.cpp
  llvm/unittests/Support/JSONTest.cpp

Index: llvm/unittests/Support/JSONTest.cpp
===
--- llvm/unittests/Support/JSONTest.cpp
+++ llvm/unittests/Support/JSONTest.cpp
@@ -173,12 +173,14 @@
   Compare(R"("\"\\\b\f\n\r\t")", "\"\\\b\f\n\r\t");
   Compare(R"("\u")", llvm::StringRef("\0", 1));
   Compare("\"\x7f\"", "\x7f");
+#ifndef __cpp_char8_t
   Compare(R"("\ud801\udc37")", u8"\U00010437"); // UTF16 surrogate pair escape.
   Compare("\"\xE2\x82\xAC\xF0\x9D\x84\x9E\"", u8"\u20ac\U0001d11e"); // UTF8
   Compare(
   R"("LoneLeading=\ud801, LoneTrailing=\udc01, LeadingLeadingTrailing=\ud801\ud801\udc37")",
   u8"LoneLeading=\ufffd, LoneTrailing=\ufffd, "
   u8"LeadingLeadingTrailing=\ufffd\U00010437"); // Invalid unicode.
+#endif
 
   Compare(R"({"":0,"":0})", Object{{"", 0}});
   Compare(R"({"obj":{},"arr":[]})", Object{{"obj", Object{}}, {"arr", {}}});
Index: llvm/unittests/Support/DJBTest.cpp
===
--- llvm/unittests/Support/DJBTest.cpp
+++ llvm/unittests/Support/DJBTest.cpp
@@ -24,6 +24,7 @@
   {{""}, {""}},
 
   {{"I"}, {"i"}},
+#ifndef __cpp_char8_t
   // Latin Small Letter Dotless I
   {{u8"\u0130"}, {"i"}},
   // Latin Capital Letter I With Dot Above
@@ -47,6 +48,7 @@
   {{u8"\uff2d"}, {u8"\uff4d"}},
   // Old Hungarian Capital Letter Ej
   {{u8"\U00010c92"}, {u8"\U00010cd2"}},
+#endif
   };
 
   for (const TestCase  : Tests) {
@@ -79,6 +81,7 @@
   }
 }
 
+#ifndef __cpp_char8_t
 TEST(DJBTest, knownValuesUnicode) {
   EXPECT_EQ(5866553u, djbHash(u8"\u0130"));
   EXPECT_EQ(177678u, caseFoldingDjbHash(u8"\u0130"));
@@ -93,3 +96,4 @@
   u8"\u0130\u0131\u00c0\u00e0\u0100\u0101\u0139\u013a\u0415\u0435\u1ea6"
   u8"\u1ea7\u212a\u006b\u2c1d\u2c4d\uff2d\uff4d\U00010c92\U00010cd2"));
 }
+#endif
\ No newline at end of file
Index: llvm/unittests/ADT/STLExtrasTest.cpp
===
--- llvm/unittests/ADT/STLExtrasTest.cpp
+++ llvm/unittests/ADT/STLExtrasTest.cpp
@@ -463,21 +463,21 @@
 
   // Check fancy pointer overload for unique_ptr
   std::unique_ptr V2 = std::make_unique(0);
-  EXPECT_EQ(V2.get(), to_address(V2));
+  EXPECT_EQ(V2.get(), (to_address)(V2));
 
   V2.reset(V1);
-  EXPECT_EQ(V1, to_address(V2));
+  EXPECT_EQ(V1, (to_address)(V2));
   V2.release();
 
   // Check fancy pointer overload for shared_ptr
   std::shared_ptr V3 = std::make_shared(0);
   std::shared_ptr V4 = V3;
   EXPECT_EQ(V3.get(), V4.get());
-  EXPECT_EQ(V3.get(), to_address(V3));
-  EXPECT_EQ(V4.get(), to_address(V4));
+  EXPECT_EQ(V3.get(), (to_address)(V3));
+  EXPECT_EQ(V4.get(), (to_address)(V4));
 
   V3.reset(V1);
-  EXPECT_EQ(V1, to_address(V3));
+  EXPECT_EQ(V1, (to_address)(V3));
 }
 
 TEST(STLExtrasTest, partition_point) {
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -149,8 +149,8 @@
   // The instruction (VN) which uses the values flowing out of CHI.
   Instruction *I;
 
-  bool operator==(const CHIArg ) { return VN == A.VN; }
-  bool operator!=(const CHIArg ) { return !(*this == A); }
+  bool operator==(const CHIArg ) const { return VN == A.VN; }
+  bool operator!=(const CHIArg ) const { return !(*this == A); }
 };
 
 using CHIIt = SmallVectorImpl::iterator;
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -665,7 +665,7 @@
   if (auto

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-05-25 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin marked 2 inline comments as done.
BRevzin added a comment.

I hadn't build the tests before, updated with a few more changes. Some of the 
tests require `u8` literals, whose type changes in C++20. I had no idea what to 
do with that, so I just `#ifdef`-ed out those tests with the appropriate 
feature test macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938



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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-04-28 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added a comment.

In D78938#2007037 , @dblaikie wrote:

> (peanut gallery: I'd consider, while you're touching these all anyway, 
> changing them all to non-member (friended where required) as I believe that's 
> best practice - allows equal implicit conversions on either side, for 
> instance (even if some types have no implicit conversions - it at least 
> provides a nice consistency/examples that people are likely to copy from))

Hidden friend is probably the best way to write comparisons in C++17 and 
earlier, but I'm not sure that will hold in C++20 (even if LLVM isn't on C++20 
and won't be for I imagine quite some time). With reversed candidates, I think 
member functions might be the way to go there - you still get implicit 
conversions on either side (just not on both sides at the same time) and hidden 
friends... are kind of weird, to be honest.

Also, I didn't touch all of them - only the ones that break in C++20 (a lot of 
which just missing a `const`). A lot of comparison operators are already fine. 
I'm not sure it's worth changing them just to look the same.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-04-27 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added a comment.

In D78938#2006758 , 
@hubert.reinterpretcast wrote:

> In D78938#2006715 , @BRevzin wrote:
>
> > Wtf, where'd my other changes go?
>
>
> I've hit this before, use `arc diff --update D78938 `.


Thanks Hubert!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938



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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-04-27 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin updated this revision to Diff 260521.
BRevzin added a comment.

Trying this again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

Files:
  clang/include/clang/AST/StmtIterator.h
  clang/lib/Parse/ParseOpenMP.cpp
  llvm/include/llvm/ADT/AllocatorList.h
  llvm/include/llvm/ADT/DenseMap.h
  llvm/include/llvm/ADT/DenseSet.h
  llvm/include/llvm/ADT/DirectedGraph.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/StringMap.h
  llvm/include/llvm/ADT/iterator.h
  llvm/include/llvm/CodeGen/LiveInterval.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/Support/BinaryStreamRef.h
  llvm/lib/CodeGen/MachineOutliner.cpp
  llvm/lib/CodeGen/PeepholeOptimizer.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp

Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -149,8 +149,8 @@
   // The instruction (VN) which uses the values flowing out of CHI.
   Instruction *I;
 
-  bool operator==(const CHIArg ) { return VN == A.VN; }
-  bool operator!=(const CHIArg ) { return !(*this == A); }
+  bool operator==(const CHIArg ) const { return VN == A.VN; }
+  bool operator!=(const CHIArg ) const { return !(*this == A); }
 };
 
 using CHIIt = SmallVectorImpl::iterator;
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -665,7 +665,7 @@
   if (auto *T = dyn_cast_or_null(Attachment.second))
 for (unsigned N = 0; N < T->getNumOperands(); ++N)
   if (auto *Loc = dyn_cast_or_null(T->getOperand(N)))
-if (Loc != DebugLoc())
+if (Loc != DebugLoc().get())
   T->replaceOperandWith(N, remapDebugLoc(Loc));
   }
 }
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -1729,7 +1729,7 @@
   return Alignment != 0;
 }
 
-bool AttrBuilder::operator==(const AttrBuilder ) {
+bool AttrBuilder::operator==(const AttrBuilder ) const {
   if (Attrs != B.Attrs)
 return false;
 
Index: llvm/lib/CodeGen/PeepholeOptimizer.cpp
===
--- llvm/lib/CodeGen/PeepholeOptimizer.cpp
+++ llvm/lib/CodeGen/PeepholeOptimizer.cpp
@@ -330,7 +330,7 @@
   return RegSrcs[Idx].SubReg;
 }
 
-bool operator==(const ValueTrackerResult ) {
+bool operator==(const ValueTrackerResult ) const {
   if (Other.getInst() != getInst())
 return false;
 
Index: llvm/lib/CodeGen/MachineOutliner.cpp
===
--- llvm/lib/CodeGen/MachineOutliner.cpp
+++ llvm/lib/CodeGen/MachineOutliner.cpp
@@ -590,10 +590,10 @@
   return It;
 }
 
-bool operator==(const RepeatedSubstringIterator ) {
+bool operator==(const RepeatedSubstringIterator ) const {
   return N == Other.N;
 }
-bool operator!=(const RepeatedSubstringIterator ) {
+bool operator!=(const RepeatedSubstringIterator ) const {
   return !(*this == Other);
 }
 
Index: llvm/include/llvm/Support/BinaryStreamRef.h
===
--- llvm/include/llvm/Support/BinaryStreamRef.h
+++ llvm/include/llvm/Support/BinaryStreamRef.h
@@ -121,12 +121,12 @@
 
   bool valid() const { return BorrowedImpl != nullptr; }
 
-  bool operator==(const RefType ) const {
-if (BorrowedImpl != Other.BorrowedImpl)
+  friend bool operator==(const RefType , const RefType ) {
+if (Self.BorrowedImpl != Other.BorrowedImpl)
   return false;
-if (ViewOffset != Other.ViewOffset)
+if (Self.ViewOffset != Other.ViewOffset)
   return false;
-if (Length != Other.Length)
+if (Self.Length != Other.Length)
   return false;
 return true;
   }
Index: llvm/include/llvm/ProfileData/InstrProfReader.h
===
--- llvm/include/llvm/ProfileData/InstrProfReader.h
+++ llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -50,8 +50,12 @@
   InstrProfIterator(InstrProfReader *Reader) : Reader(Reader) { Increment(); }
 
   InstrProfIterator ++() { Increment(); return *this; }
-  bool operator==(const InstrProfIterator ) { return Reader == RHS.Reader; }
-  bool operator!=(const InstrProfIterator ) { return Reader != RHS.Reader; }
+  bool operator==(const InstrProfIterator ) const {
+return Reader == RHS.Reader;
+  }
+  bool operator!=(const InstrProfIterator ) const {
+return Reader != RHS.Reader;
+  }
   value_type *() {

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-04-27 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added a comment.

Wtf, where'd my other changes go?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938



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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-04-27 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin marked 2 inline comments as done.
BRevzin added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:69-70
+  }
   bool operator==(OpenMPDirectiveKind V) const { return Value == unsigned(V); }
   bool operator!=(OpenMPDirectiveKind V) const { return Value != unsigned(V); }
   bool operator<(OpenMPDirectiveKind V) const { return Value < unsigned(V); }

rsmith wrote:
> Do we still need these?
Yep. This type is compared to both itself and the enum, so both are necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938



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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-04-27 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin updated this revision to Diff 260477.
BRevzin added a comment.

More idiomatic comparison implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

Files:
  clang/lib/Parse/ParseOpenMP.cpp


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -61,10 +61,10 @@
   OpenMPDirectiveKindExWrapper(unsigned Value) : Value(Value) {}
   OpenMPDirectiveKindExWrapper(OpenMPDirectiveKind DK) : Value(unsigned(DK)) {}
   bool operator==(OpenMPDirectiveKindExWrapper V) const {
-return Value == unsigned(V);
+return Value == V.Value;
   }
   bool operator!=(OpenMPDirectiveKindExWrapper V) const {
-return Value != unsigned(V);
+return Value != V.Value;
   }
   bool operator==(OpenMPDirectiveKind V) const { return Value == unsigned(V); }
   bool operator!=(OpenMPDirectiveKind V) const { return Value != unsigned(V); }


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -61,10 +61,10 @@
   OpenMPDirectiveKindExWrapper(unsigned Value) : Value(Value) {}
   OpenMPDirectiveKindExWrapper(OpenMPDirectiveKind DK) : Value(unsigned(DK)) {}
   bool operator==(OpenMPDirectiveKindExWrapper V) const {
-return Value == unsigned(V);
+return Value == V.Value;
   }
   bool operator!=(OpenMPDirectiveKindExWrapper V) const {
-return Value != unsigned(V);
+return Value != V.Value;
   }
   bool operator==(OpenMPDirectiveKind V) const { return Value == unsigned(V); }
   bool operator!=(OpenMPDirectiveKind V) const { return Value != unsigned(V); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-04-27 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, hiraditya, 
MatzeB.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, LLVM.

Part of the <=> changes in C++20 make certain patterns of writing equality 
operators ambiguous with themselves (sorry!). This review goes through and 
adjusts all the comparison oeprators such that they should work in both C++17 
and C++20 modes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78938

Files:
  clang/include/clang/AST/StmtIterator.h
  clang/lib/Parse/ParseOpenMP.cpp
  llvm/include/llvm/ADT/AllocatorList.h
  llvm/include/llvm/ADT/DenseMap.h
  llvm/include/llvm/ADT/DenseSet.h
  llvm/include/llvm/ADT/DirectedGraph.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/StringMap.h
  llvm/include/llvm/ADT/iterator.h
  llvm/include/llvm/CodeGen/LiveInterval.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/Support/BinaryStreamRef.h
  llvm/lib/CodeGen/MachineOutliner.cpp
  llvm/lib/CodeGen/PeepholeOptimizer.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp

Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -149,8 +149,8 @@
   // The instruction (VN) which uses the values flowing out of CHI.
   Instruction *I;
 
-  bool operator==(const CHIArg ) { return VN == A.VN; }
-  bool operator!=(const CHIArg ) { return !(*this == A); }
+  bool operator==(const CHIArg ) const { return VN == A.VN; }
+  bool operator!=(const CHIArg ) const { return !(*this == A); }
 };
 
 using CHIIt = SmallVectorImpl::iterator;
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -665,7 +665,7 @@
   if (auto *T = dyn_cast_or_null(Attachment.second))
 for (unsigned N = 0; N < T->getNumOperands(); ++N)
   if (auto *Loc = dyn_cast_or_null(T->getOperand(N)))
-if (Loc != DebugLoc())
+if (Loc != DebugLoc().get())
   T->replaceOperandWith(N, remapDebugLoc(Loc));
   }
 }
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -1729,7 +1729,7 @@
   return Alignment != 0;
 }
 
-bool AttrBuilder::operator==(const AttrBuilder ) {
+bool AttrBuilder::operator==(const AttrBuilder ) const {
   if (Attrs != B.Attrs)
 return false;
 
Index: llvm/lib/CodeGen/PeepholeOptimizer.cpp
===
--- llvm/lib/CodeGen/PeepholeOptimizer.cpp
+++ llvm/lib/CodeGen/PeepholeOptimizer.cpp
@@ -330,7 +330,7 @@
   return RegSrcs[Idx].SubReg;
 }
 
-bool operator==(const ValueTrackerResult ) {
+bool operator==(const ValueTrackerResult ) const {
   if (Other.getInst() != getInst())
 return false;
 
Index: llvm/lib/CodeGen/MachineOutliner.cpp
===
--- llvm/lib/CodeGen/MachineOutliner.cpp
+++ llvm/lib/CodeGen/MachineOutliner.cpp
@@ -590,10 +590,10 @@
   return It;
 }
 
-bool operator==(const RepeatedSubstringIterator ) {
+bool operator==(const RepeatedSubstringIterator ) const {
   return N == Other.N;
 }
-bool operator!=(const RepeatedSubstringIterator ) {
+bool operator!=(const RepeatedSubstringIterator ) const {
   return !(*this == Other);
 }
 
Index: llvm/include/llvm/Support/BinaryStreamRef.h
===
--- llvm/include/llvm/Support/BinaryStreamRef.h
+++ llvm/include/llvm/Support/BinaryStreamRef.h
@@ -121,12 +121,12 @@
 
   bool valid() const { return BorrowedImpl != nullptr; }
 
-  bool operator==(const RefType ) const {
-if (BorrowedImpl != Other.BorrowedImpl)
+  friend bool operator==(const RefType , const RefType ) {
+if (Self.BorrowedImpl != Other.BorrowedImpl)
   return false;
-if (ViewOffset != Other.ViewOffset)
+if (Self.ViewOffset != Other.ViewOffset)
   return false;
-if (Length != Other.Length)
+if (Self.Length != Other.Length)
   return false;
 return true;
   }
Index: llvm/include/llvm/ProfileData/InstrProfReader.h
===
--- llvm/include/llvm/ProfileData/InstrProfReader.h
+++ llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -50,8 +50,12 @@
   InstrProfIterator(InstrProfReader *Reader) : Reader(Reader) { Increment(); }
 
   InstrProfIterator ++() { Increment(); return *this; }
-  bool operator==(const InstrProfIterator ) { return Reader == RHS.Reader; }
-  bool

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

2018-01-05 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin marked an inline comment as done.
BRevzin added a comment.

In https://reviews.llvm.org/D41740#968134, @JonasToth wrote:

> Could you please add a test case with a template that reduces the type to 
> int8 or uint8?

I don't actually know how to do that. I tried a few things, but getting the 
type of the expression through a template gets me directly to `unsigned char`, 
not to `uint8_t`.

https://reviews.llvm.org/D41740

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

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

2018-01-05 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin updated this revision to Diff 128768.
BRevzin added a comment.

Updates based on review comments  - and rebased off of latest so as to get the 
ReleaseNotes right.  Added options so that the user can provide the list of 
stream types and int typedef types, as desired, defaulting to just 
`basic_ostream` and `int8_t`/`uint8_t`.


https://reviews.llvm.org/D41740

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/StreamInt8Check.cpp
  clang-tidy/bugprone/StreamInt8Check.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-stream-int8.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-stream-int8.cpp

Index: test/clang-tidy/bugprone-stream-int8.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-stream-int8.cpp
@@ -0,0 +1,46 @@
+// RUN: %check_clang_tidy %s bugprone-stream-int8 %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: bugprone-stream-int8.AliasTypes, \
+// RUN:   value: 'int8_t; uint8_t; widget'}]}" \
+// RUN:   -- -std=c++11
+
+using int8_t = signed char;
+using uint8_t = unsigned char;
+using widget = unsigned char;
+
+namespace std {
+template 
+struct basic_ostream;
+
+using ostream = basic_ostream;
+
+basic_ostream& operator<<(basic_ostream&, char );
+basic_ostream& operator<<(basic_ostream&, unsigned char );
+basic_ostream& operator<<(basic_ostream&, signed char );
+basic_ostream& operator<<(basic_ostream&, int );
+}
+
+void BadStreaming(std::ostream& os, uint8_t i) {
+// CHECK-MESSAGES: :[[@LINE+1]]:11: warning: streaming uint8_t; did you mean to stream an int? [bugprone-stream-int8]
+os << i;
+
+// CHECK-MESSAGES: :[[@LINE+1]]:16: warning: streaming uint8_t; did you mean to stream an int? [bugprone-stream-int8]
+os << 4 << i;
+
+// CHECK-MESSAGES: :[[@LINE+1]]:11: warning: streaming int8_t; did you mean to stream an int? [bugprone-stream-int8]
+os << int8_t{};
+
+using Num = int8_t;
+// CHECK-MESSAGES: :[[@LINE+1]]:11: warning: streaming int8_t; did you mean to stream an int? [bugprone-stream-int8]
+os << Num{};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:11: warning: streaming widget; did you mean to stream an int? [bugprone-stream-int8]
+os << widget{};
+}
+
+void GoodStreaming(std::ostream& os, uint8_t i) {
+using UC = unsigned char;
+
+unsigned char uc;
+os << +i << 4 << uc << UC{};
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -29,6 +29,7 @@
bugprone-misplaced-operator-in-strlen-in-alloc
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
+   bugprone-stream-int8
bugprone-string-constructor
bugprone-suspicious-memset-usage
bugprone-undefined-memory-manipulation
Index: docs/clang-tidy/checks/bugprone-stream-int8.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-stream-int8.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - bugprone-stream-int8
+
+bugprone-stream-int8
+
+
+Checks that objects of type ``int8_t`` or ``uint8_t`` aren't streamed,  if those
+are typedefs to ``signed char`` or ``unsigned char``, as this likely intends to
+be streaming these types as ``int`` s instead.
+
+Examples:
+
+.. code-block:: c++
+
+  uint8_t value = 0;
+  std::cout << "Value is " << value; // prints ^@ instead of likely intended 0
+
+Options
+---
+
+.. option:: StreamTypes
+
+   A semicolon-separated list of class names that should be treated as streams.
+   By default only ``std::basic_ostream`` is considered.
+
+.. option:: AliasTypes
+
+   A semicolon-separated list of alias names that should be treated as integer
+   types to check against. By default only ``int8_t`` and ``uint8_t`` are
+   considered.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,12 @@
 Improvements to clang-tidy
 --
 
-- ...
+- New `bugprone-stream-int8_t
+  `_ check
+
+  Checks that objects of type ``int8_t`` or ``uint8_t`` aren't streamed,  if those
+  are typedefs to ``signed char`` or ``unsigned char``, as this likely intends to
+  be streaming these types as ``int`` s instead.
 
 Improvements to include-fixer
 -
Index: clang-tidy/bugprone/StreamInt8Check.h
===
--- /dev/null
+++ clang-tidy/bugprone/StreamInt8Check.h
@@ -0,0 +1,43 @@
+//===--- StreamInt8Check.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

2018-01-04 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added a comment.

So admittedly, I don't have any experience in clang-tidy. This check is 
intended to catch what is to us a really annoying source of error (and I'm 
probably not the only one?). I suspect there are far better ways of writing 
this check than the one that I pretty much guess-and-checked my way into.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41740



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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[PATCH] D78938: Make LLVM build in C++20 mode

[PATCH] D78938: Make LLVM build in C++20 mode

[PATCH] D78938: Make LLVM build in C++20 mode

[PATCH] D78938: Make LLVM build in C++20 mode

[PATCH] D78938: Make LLVM build in C++20 mode

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

26 matches

Site Navigation

Mail list logo

Footer information