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

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

On C++20 mode rotting: it won't if someone sets up a bot. If it rots, then it's 
easier to un-rot with Barry's patch.




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:
> BRevzin wrote:
> > 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. 
> Could it be a pre-requisite patch then?
I'm fine with this if the patch title is changed to "make LLVM build in C++20 
mode", and description edited accordingly. Basically, it makes it easy to 
figure out which changes were done for C++20.


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.

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

In D78938#2259342 , @BRevzin wrote:

> 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.

Makes sense, thanks.




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";

BRevzin wrote:
> 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. 
Could it be a pre-requisite patch then?


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.

[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.

[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.

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

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.




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";

This seems unrelated to comparison checking?



Comment at: llvm/unittests/ADT/STLExtrasTest.cpp:465
   // 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);

Nit: trailing full stop.


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.

[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.

[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.

[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.

[David Stone via Phabricator via cfe-commits]

davidstone added a comment.

I noticed the missing return because there is a warning (not as error) that 
caught it, I think the warning about falling off the end of a 
non-void-returning function.


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.

[JF Bastien via Phabricator via cfe-commits]

jfb accepted this revision.
jfb added a comment.

One suggestions, otherwise looks good. Thanks for doing this :)




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

davidstone wrote:
> Missing `return`


Did this not trigger a diagnostic when building? I wonder if it's just not on?



Comment at: llvm/include/llvm/ADT/DirectedGraph.h:40
   /// Static polymorphism: delegate implementation (via isEqualTo) to the
   /// derived class.
+  bool operator==(const DGEdge ) const {

That comment, so informative! 



Comment at: llvm/unittests/ADT/STLExtrasTest.cpp:466
   std::unique_ptr V2 = std::make_unique(0);
-  EXPECT_EQ(V2.get(), to_address(V2));
+  EXPECT_EQ(V2.get(), (to_address)(V2));
 

Can you add a comment above (with "fancy pointer") so mere mortals understand 
the parens?


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.

[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.

[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.

[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.

[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.

[David Stone via Phabricator via cfe-commits]

davidstone added inline comments.
Herald added a subscriber: sstefan1.



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

Missing `return`


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.

[JF Bastien via Phabricator via cfe-commits]

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

This seems fine, assuming you've run usual 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.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D78938#2007571 , @BRevzin wrote:

> 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.


Yeah, probably just experience with things the way they've been, but the 
symmetry is kind of nice without relying on deeper aspects of the newer 
features (& the benefit of the code being more suitable for C++17, where LLVM 
is currently).

> 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.

Yeah - just meant the ones you are touching, might be nice to move them in that 
direction.

Anyway, I'll leave it to you/other reviewers - no /super/ strong feelings here.


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.

[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.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

(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))


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.

[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.

[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.

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D78938#2006715 , @BRevzin wrote:

> Wtf, where'd my other changes go?


I've hit this before, use `arc diff --update D78938 `.


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.

[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.

[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.

[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.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:64-67
+return Value == unsigned(V);
+  }
+  bool operator!=(OpenMPDirectiveKindExWrapper V) const {
+return Value != unsigned(V);

Comparing against `V.Value` here would seem more idiomatic than invoking 
`operator unsigned`.



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); }

Do we still need these?


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.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: llvm/include/llvm/Support/BinaryStreamRef.h:124
 
-  bool operator==(const RefType ) const {
-if (BorrowedImpl != Other.BorrowedImpl)
+  friend bool operator==(const RefType , const RefType ) {
+if (Self.BorrowedImpl != Other.BorrowedImpl)

Is there a neat rule of thumb for when you were able to preserve the member 
`bool Me::operator==(const Me& rhs) const` versus when you were forced to 
change it to a hidden friend? It seems like maybe you changed it to a hidden 
friend only in cases where `Me` was a base class, is that right?


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.

[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