[clang] Add `clang::equality_operator_compares_members_lexicographically`

[philnik]

Summary

This attribute is equivalent to a defaulted member equality comparison for the purposes of __is_trivially_equality_comparable. The difference is that it doesn't try to do ADL calls when instantiating a class template. We encountered this problem while trying to make classes in libc++ trivially equality comparable. It can be used to guarantee that enums are trivially equality comparable (which is more of a bonus than a good reason to add this attribute).

[philnik]

If anybody has a better name for this I'm all ears.

[erichkeane]

I'm a little dense today perhaps, but I don't get the use case for this? Can you ELI-EWG? Is this an attribute we expect our users to use? The name is horrifyingly long for users to use, but perhaps that is a good thing.

clang/include/clang/Basic/AttrDocs.td

541–542

Unrelated changes. I realize your editor probably does this, please don't have it do it on this file.

3531

[philnik]

In D151625#4380735, @erichkeane wrote:

I'm a little dense today perhaps, but I don't get the use case for this? Can you ELI-EWG? Is this an attribute we expect our users to use? The name is horrifyingly long for users to use, but perhaps that is a good thing.

This is useful for libraries which have ADL requirements. Something like

template <class T>
struct Holder {
  T v;
};

struct Incomplete;

template <class T>
class TupleImpl {
  T v;

  bool operator==(const TupleImpl&) const = default;
};

template <class T>
class Tuple : public TupleImpl<T> {
  bool operator==(const Tuple&) const = default; // ADL call here
};

void func() {
  Tuple<Holder<Incomplete>*> f; // Instantiation fails because Incomplete isn't defined
}

has to work for std::tuple etc. to make them __is_trivially_equality_comparable, but the defaulted operator== will always try ADL (at least that's my understanding). To not make ADL calls, this attribute can be used instead, which basically say "trust me, my operator== does what the defaulted one would do".

Not sure what you mean by "Can you ELI-EWG?".
I agree that the name isn't great, but I couldn't come up with anything better that is still descriptive, and this attribute will probably only be used by very few libraries anyways (maybe just libc++, since this is pure QoI). As I said in the attribute description, the defaulted operator should be used if possible (which is probably the case in 99.9% of use cases).

(Note to self: maybe add the example as a test)

[erichkeane]

In D151625#4382227, @philnik wrote:

In D151625#4380735, @erichkeane wrote:

I'm a little dense today perhaps, but I don't get the use case for this? Can you ELI-EWG? Is this an attribute we expect our users to use? The name is horrifyingly long for users to use, but perhaps that is a good thing.

This is useful for libraries which have ADL requirements. Something like

template <class T>
struct Holder {
  T v;
};

struct Incomplete;

template <class T>
class TupleImpl {
  T v;

  bool operator==(const TupleImpl&) const = default;
};

template <class T>
class Tuple : public TupleImpl<T> {
  bool operator==(const Tuple&) const = default; // ADL call here
};

void func() {
  Tuple<Holder<Incomplete>*> f; // Instantiation fails because Incomplete isn't defined
}

has to work for std::tuple etc. to make them __is_trivially_equality_comparable, but the defaulted operator== will always try ADL (at least that's my understanding). To not make ADL calls, this attribute can be used instead, which basically say "trust me, my operator== does what the defaulted one would do".

Not sure what you mean by "Can you ELI-EWG?".
I agree that the name isn't great, but I couldn't come up with anything better that is still descriptive, and this attribute will probably only be used by very few libraries anyways (maybe just libc++, since this is pure QoI). As I said in the attribute description, the defaulted operator should be used if possible (which is probably the case in 99.9% of use cases).

(Note to self: maybe add the example as a test)

ELI-EWG: "Explain-like-I'm Evolution Working Group" :) Its a touch of a joke, there is a common reddit 'ELI5' (Explain like I'm 5), so I'm equating the C++ EWG to needing 5-year-old-esque explanations.

I can see the value to that I guess? I don't have a good idea on the name, but perhaps folks from @clang-language-wg might have an idea? Either way, I want to give it a little time for folks to comment.

That said, it DOES seem that the pre-commit-CI failures are related, so those need fixing as well.

[philnik]

• Address comments
• Try to fix CI

[erichkeane]

1- This needs a release note.
2- I want others to comment on the name. I like its verbosity in that it'll discourage people from using it without knowing what it means, but I want to hear Aaron/others' opinion as well.
3- Trailing WS thing, I see you did a cleanup patch, so just make sure this gets commited after that.

clang/include/clang/Basic/AttrDocs.td

541

Trailing WS change, please don't do in this commit.

[philnik]

@aaron.ballman Do you have any opinion here?

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

3525

The docs don't make it clear what the ramifications are of using the attribute. Do you get better codegen? Different diagnostics? That sort of thing.

It'd probably help for the docs to have a short code example showing what the difference the attribute makes.

Also, should we explicitly define it as UB if the user applies the attribute to something and the equivalence requirements aren't met?

If the attribute is written on a structure that's used as a base class, does the derived class automatically "inherit" the behavior as well? Or do derived classes have to be redeclared with the attribute? Is it reasonable for a base class which does not have the attribute to be inherited by a derived class which adds the attribute?

(I'm kind of wondering whether there are situations in which we want to diagnose use of the attribute to help programmers avoid mistakes.)

3531–3533

clang/test/SemaCXX/attr-equality-operator-compares-members-lexicographically.cpp

1

Is the triple necessary?

72

FIXME?

[AMP999]

This attribute is trying to work around an issue with operator== instantiation, but according to https://cpplang.slack.com/archives/C6K47F8TT/p1689107603335019?thread_ts=1689107603.335019&cid=C6K47F8TT the problem is actually a bug in Clang. Shouldn't the Clang bug be fixed instead, so that this attribute won't be needed?

[aaron.ballman]

In D151625#4528817, @AMP999 wrote:

This attribute is trying to work around an issue with operator== instantiation, but according to https://cpplang.slack.com/archives/C6K47F8TT/p1689107603335019?thread_ts=1689107603.335019&cid=C6K47F8TT the problem is actually a bug in Clang. Shouldn't the Clang bug be fixed instead, so that this attribute won't be needed?

Any chance you can provide some more details about why the problem is a bug in Clang (Slack seems to require signing up in order to see what's behind the link)?

[AMP999]

Any chance you can provide some more details about why the problem is a bug in Clang (Slack seems to require signing up in order to see what's behind the link)?

Sure, I just filed this as a GitHub issue in the following link: https://github.com/llvm/llvm-project/issues/64124

[aaron.ballman]

In D151625#4534850, @AMP999 wrote:

Any chance you can provide some more details about why the problem is a bug in Clang (Slack seems to require signing up in order to see what's behind the link)?

Sure, I just filed this as a GitHub issue in the following link: https://github.com/llvm/llvm-project/issues/64124

Thank you for filing it as an issue, that's helpful!

clang/include/clang/Basic/Attr.td

def TrivialABI : InheritableAttr {
   // CPlusPlus language option.
   let Spellings = [Clang<"trivial_abi", 0>];
   let Subjects = SubjectList<[CXXRecord]>;
   let Documentation = [TrivialABIDocs];
   let LangOpts = [CPlusPlus];
   let SimpleHandler = 1;
 }
 
+def EqualityOperatorComparesMembersLexicographically : InheritableAttr {
+  let Spellings = [Clang<"equality_operator_compares_members_lexicographically", 0>];
+  let Subjects = SubjectList<[CXXRecord, Enum]>;
+  let Documentation = [EqualityOperatorComparesMembersLexicographicallyDocs];
+  let LangOpts = [CPlusPlus];
+  let SimpleHandler = 1;
+}
+
 def MaxFieldAlignment : InheritableAttr {
   // This attribute has no spellings as it is only ever created implicitly.
   let Spellings = [];
   let Args = [UnsignedArgument<"Alignment">];
   let SemaHandler = 0;
   let Documentation = [InternalOnly];
 }
 

clang/include/clang/Basic/AttrDocs.td

 The ``[[_Noreturn]]`` spelling is deprecated and only exists to ease code
 migration for code using ``[[noreturn]]`` after including ``<stdnoreturn.h>``.
   }];
 }
 
 def NoMergeDocs : Documentation {
   let Category = DocCatStmt;
   let Content = [{
-If a statement is marked ``nomerge`` and contains call expressions, those call
-expressions inside the statement will not be merged during optimization. This 
+If a statement is marked ``nomerge`` and contains call expressions, those call 
+expressions inside the statement will not be merged during optimization. This
 attribute can be used to prevent the optimizer from obscuring the source
 location of certain calls. For example, it will prevent tail merging otherwise
 identical code sequences that raise an exception or terminate the program. Tail
 merging normally reduces the precision of source location information, making
 stack traces less useful for debugging. This attribute gives the user control
 over the tradeoff between code size and debug information precision. 
 
 ``nomerge`` attribute can also be used as function attribute to prevent all 

 
 ``tvos``
   Apple's tvOS operating system. The minimum deployment target is specified by
   the ``-mtvos-version-min=*version*`` command-line argument.
 
 ``watchos``
   Apple's watchOS operating system. The minimum deployment target is specified by
   the ``-mwatchos-version-min=*version*`` command-line argument.
-  
 ``driverkit``
   Apple's DriverKit userspace kernel extensions. The minimum deployment target
   is specified as part of the triple.
 
 A declaration can typically be used even when deploying back to a platform
 version prior to when the declaration was introduced. When this happens, the
 declaration is `weakly linked
 <https://developer.apple.com/library/mac/#documentation/MacOSX/Conceptual/BPFrameworks/Concepts/WeakLinking.html>`_,
- The class has a non-static data member whose type is non-trivial for the
   purposes of calls, which includes:
 
   - classes that are non-trivial for the purposes of calls
   - __weak-qualified types in Objective-C++
   - arrays of any of the above
   }];
 }
 
+def EqualityOperatorComparesMembersLexicographicallyDocs : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+On classes, the ``equality_operator_compares_members_lexicographically``
+attribute informs clang that a call to ``operator==(const T&, const T&)`` is
+equivalent to comparing the members lexicographically (i.e. the call is
+equivalent to a call to ``bool operator==(const T&) const = default``). This is
+useful for libraries which have strict requirements on ADL uses. If possible, a
+defaulted equality comparison operator should be preferred over this attribute.
+On enums, the attribute guarantees that there are either no custom comparison
+operators defined, or any that defined are quivalent to the builtin comparison
+operator.
+  }];
+}
+
 def MSInheritanceDocs : Documentation {
   let Category = DocCatDecl;
   let Heading = "__single_inhertiance, __multiple_inheritance, __virtual_inheritance";
   let Content = [{
 This collection of keywords is enabled under ``-fms-extensions`` and controls
 the pointer-to-member representation used on ``*-*-win32`` targets.
 
 The ``*-*-win32`` targets utilize a pointer-to-member representation which

clang/lib/AST/Type.cpp

HasNonDeletedDefaultedEqualityComparison(const CXXRecordDecl *Decl) {
   auto IsDefaultedOperatorEqualEqual = [&](const FunctionDecl *Function) {
     return Function->getOverloadedOperator() ==
                OverloadedOperatorKind::OO_EqualEqual &&
            Function->isDefaulted() && Function->getNumParams() > 0 &&
            (Function->getParamDecl(0)->getType()->isReferenceType() ||
             Decl->isTriviallyCopyable());
   };
 
-  if (llvm::none_of(Decl->methods(), IsDefaultedOperatorEqualEqual) &&
+  if (!Decl->hasAttr<EqualityOperatorComparesMembersLexicographicallyAttr>() &&
+      llvm::none_of(Decl->methods(), IsDefaultedOperatorEqualEqual) &&
       llvm::none_of(Decl->friends(), [&](const FriendDecl *Friend) {
         if (NamedDecl *ND = Friend->getFriendDecl()) {
           return ND->isFunctionOrFunctionTemplate() &&
                  IsDefaultedOperatorEqualEqual(ND->getAsFunction());
         }
         return false;
       }))
     return false;
 
   return llvm::all_of(Decl->bases(),
                       [](const CXXBaseSpecifier &BS) {
                         if (const auto *RD = BS.getType()->getAsCXXRecordDecl())
                           HasNonDeletedDefaultedEqualityComparison(RD);
                         return true;
                       }) &&
          llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
            auto Type = FD->getType();
-           if (Type->isReferenceType() || Type->isEnumeralType())
+           if (Type->isReferenceType() ||
+               (Type->isEnumeralType() &&
+                !Type->getAsTagDecl()
+                     ->hasAttr<
+                         EqualityOperatorComparesMembersLexicographicallyAttr>()))
              return false;
            if (const auto *RD = Type->getAsCXXRecordDecl())
              return HasNonDeletedDefaultedEqualityComparison(RD);
            return true;
          });
 }
 
 bool QualType::isTriviallyEqualityComparableType(
     const ASTContext &Context) const {
   QualType CanonicalType = getCanonicalType();
   if (CanonicalType->isIncompleteType() || CanonicalType->isDependentType() ||
-      CanonicalType->isEnumeralType())
+      (CanonicalType->isEnumeralType() &&
+       !CanonicalType->getAsTagDecl()
+            ->hasAttr<EqualityOperatorComparesMembersLexicographicallyAttr>()))
     return false;
 
   if (const auto *RD = CanonicalType->getAsCXXRecordDecl()) {
     if (!HasNonDeletedDefaultedEqualityComparison(RD))
       return false;
   }
 
   return Context.hasUniqueObjectRepresentations(

clang/test/Misc/pragma-attribute-supported-attributes-list.test

 // CHECK-NEXT: SwiftNewType (SubjectMatchRule_type_alias)
 // CHECK-NEXT: SwiftObjCMembers (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: TLSModel (SubjectMatchRule_variable_is_thread_local)
 // CHECK-NEXT: Target (SubjectMatchRule_function)
 // CHECK-NEXT: TargetClones (SubjectMatchRule_function)
 // CHECK-NEXT: TargetVersion (SubjectMatchRule_function)
 // CHECK-NEXT: TestTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: TrivialABI (SubjectMatchRule_record)
+// CHECK-NEXT: EqualityOperatorComparesMembersLexicographically (SubjectMatchRule_function)
 // CHECK-NEXT: Uninitialized (SubjectMatchRule_variable_is_local)
 // CHECK-NEXT: UnsafeBufferUsage (SubjectMatchRule_function)
 // CHECK-NEXT: UseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: VecReturn (SubjectMatchRule_record)
 // CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)
 // CHECK-NEXT: WarnUnused (SubjectMatchRule_record)
 // CHECK-NEXT: WarnUnusedResult (SubjectMatchRule_objc_method, SubjectMatchRule_enum, SubjectMatchRule_record, SubjectMatchRule_hasType_functionType, SubjectMatchRule_type_alias)
 // CHECK-NEXT: Weak (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)

clang/test/SemaCXX/attr-equality-operator-compares-members-lexicographically.cpp

+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify %s -std=c++20
+
+void __attribute__((equality_operator_compares_members_lexicographically)) foo(); // expected-warning {{'equality_operator_compares_members_lexicographically' attribute only applies to classes and enums}}
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparabe1 {
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparabe1));
+
+struct __attribute__((equality_operator_compares_members_lexicographically)) TriviallyEqualityComparabe2 {
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparabe2));
+
+struct [[clang::equality_operator_compares_members_lexicographically(1)]] TestMessage {}; // expected-error {{'equality_operator_compares_members_lexicographically' attribute takes no arguments}}
+
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasPadding {
+  short i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasPadding), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasFloat {
+  float i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasFloat), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasTailPadding {
+  int i;
+  char j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasTailPadding), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableBase : NotTriviallyEqualityComparableHasTailPadding {
+  char j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBase), "");
+
+class [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparablePaddedOutBase {
+  int i;
+  char c;
+};
+static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOutBase), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparablePaddedOut : TriviallyEqualityComparablePaddedOutBase {
+  char j[3];
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOut), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparable1 {
+  char i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable1));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparable2 {
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable2));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableTriviallyEqualityComparableBases
+    : TriviallyEqualityComparable1, TriviallyEqualityComparable2 {
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableTriviallyEqualityComparableBases));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableBitfield {
+  int i : 1;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBitfield));
+
+// TODO: This is trivially equality comparable
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableBitfieldFilled {
+  char i : __CHAR_BIT__;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBitfield));
+
+union [[clang::equality_operator_compares_members_lexicographically]] U {
+  int i;
+};
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableImplicitlyDeletedOperatorByUnion {
+  U u;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableImplicitlyDeletedOperatorByUnion));
+
+struct NotTriviallyEqualityComparableExplicitlyDeleted {
+  int i;
+
+  friend bool operator==(const NotTriviallyEqualityComparableExplicitlyDeleted&, const NotTriviallyEqualityComparableExplicitlyDeleted&) = delete;
+};
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableImplicitlyDeletedOperatorByStruct {
+  NotTriviallyEqualityComparableExplicitlyDeleted u;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableImplicitlyDeletedOperatorByStruct));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasReferenceMember {
+  int& i;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasReferenceMember));
+
+enum E {
+  a,
+  b
+};
+bool operator==(E, E) { return false; }
+static_assert(!__is_trivially_equality_comparable(E));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasEnum {
+  E e;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasEnum));
+
+enum [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparableEnum {
+  c,
+  d,
+};
+bool operator==(TriviallyEqualityComparableEnum, TriviallyEqualityComparableEnum);
+
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableEnum));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparableHasTriviallyEqualityComparableEnum {
+  TriviallyEqualityComparableEnum e;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableHasTriviallyEqualityComparableEnum));
+
+enum class [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparableEnumClass {
+  e,
+  f,
+};
+bool operator==(TriviallyEqualityComparableEnumClass, TriviallyEqualityComparableEnumClass);
+
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableEnumClass));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparableHasTriviallyEqualityComparableEnumClass {
+  TriviallyEqualityComparableEnumClass e;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableHasTriviallyEqualityComparableEnumClass));