[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D147175#4246513 <https://reviews.llvm.org/D147175#4246513>, @aaron.ballman wrote:

> Ah, I like this approach -- it keeps things roughly in sync with checking for unique object representations, which is great. I spotted a few questions, but in general this is heading in the right direction. You should also add a release note to clang/docs/ReleaseNotes.rst so that users know there's a new builtin coming.
>
> I suppose one thing to double-check: do you know of any other STL implementations that might be using this identifier as part of their implementation details? (We've had issues in the past where we accidentally stole a reserved identifier that was being used by libstdc++ and it caused issues.)

Neither libstdc++ nor the MSVC STL use the `__is_trivially_equality_comparable` AFAICT. A sourcegraph search also exclusively finds libc++: https://sourcegraph.com/search?q=context%3Aglobal+__is_trivially_equality_comparable&patternType=standard&sm=1&groupBy=repo (which only started using it in this release cycle, so it shouldn't be a problem).



================
Comment at: clang/lib/AST/Type.cpp:2598-2599
+static bool HasDefaultedEqualityComparison(const CXXRecordDecl *Decl) {
+  if (Decl->isUnion())
+    return false;
+
----------------
aaron.ballman wrote:
> Hmmm, is this correct? I thought there was a defaulted equality comparison operator in this case, but it's defined as deleted.
> 
> http://eel.is/c++draft/class.compare.default#2
> 
> Perhaps this function is looking for a usable defaulted equality comparison operator and not just "does it have one at all"?
Yes, this is looking for a usable one. I've renamed it. (Maybe someone has an idea for a better name?)


================
Comment at: clang/lib/AST/Type.cpp:2616-2617
+                      }) &&
+         llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
+           if (!FD->getType()->isRecordType())
+             return true;
----------------
aaron.ballman wrote:
> Do we have to look for fields with references per http://eel.is/c++draft/class.compare.default#2 ?
Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175