rsmith added a comment.
Thanks!
I think you may be missing an implementation of this rule:
> A definition of a comparison operator as defaulted that appears in a class
> shall be the first declaration of that function.
In particular, this is now invalid:
struct A;
bool operator==(A, A);
struct A {
friend bool operator==(A, A) = default; // error, not first declaration
};
As a subtle detail, I'm also not sure whether we handle the following rule
properly, and I'd like to see some tests:
> A comparison operator function for class C that is defaulted on its first
> declaration and is not defined as deleted is implicitly defined when it is
> odr-used or needed for constant evaluation.
In particular, a comparison function that's defaulted on its first declaration
is defined when it's used, but if it's defaulted on a second or subsequent
declaration then it's immediately defined:
template<char C> struct Broken {
template<typename T = void> bool operator==(Broken, Broken) { return
T::result; }
};
struct A { Broken<'A'> b; };
bool operator==(A, A) = default; // ok, does not try to define the function
until it's used
struct B { Broken<'B'> b; };
bool operator==(B, B);
bool operator==(B, B) = default; // error, immediately defined, resulting in
instantiation of Broken<'B'>::operator==, resulting in a hard error
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8368-8373
+ CXXRecordDecl *RD =
+ dyn_cast<CXXRecordDecl>(FD->getCanonicalDecl()->getLexicalDeclContext());
- CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(FD->getLexicalDeclContext());
- assert(RD && "defaulted comparison is not defaulted in a class");
+ if (!RD) {
+ RD = dyn_cast<CXXRecordDecl>(FD->getLexicalDeclContext());
+ }
----------------
I don't think you can do the "member or friend" check this way. P2085R0 doesn't
require either the defaulted declaration or the first declaration to be within
the class, and it doesn't matter which class contains the first declaration, if
any. For example, this is valid:
```
struct A;
class B {
friend bool operator==(A, A); // first declaration not in class A
bool operator==(B);
};
class A {
friend bool operator==(A, A);
B b;
};
// ok, can access both A::b and B::operator==.
bool operator==(A, A) = default; // defaulted declaration not in class A
```
I think there are two reasonable options here:
1) First work out the class type for which we're defaulting a comparison and
then traverse the list of redeclarations of the function looking for one that's
lexically within the class, or
2) First work out the class type for which we're defaulting a comparison and
then check that the function is either a member of that class or is in the list
of that class's friends.
Either approach seems fine to me.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8614-8615
+
+ CXXRecordDecl *RD =
+ cast<CXXRecordDecl>(FD->getCanonicalDecl()->getLexicalDeclContext());
SourceLocation BodyLoc =
----------------
This is not necessarily the right class. The first declaration might not
lexically be in a class at all, in a case like:
```
struct A;
bool operator==(A, A);
struct A {
friend bool operator==(A, A);
};
bool operator==(A, A) = default;
```
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:765
+ diag::note_comparison_synthesized_at)
+ << (int)DFK.asComparison() << FD;
+ }
----------------
Passing in a function declaration here doesn't seem appropriate; this will
diagnose "in defaulted equality comparison operator for 'operator==' first
required here", when we wanted to say "in defaulted equality comparison
operator for 'T' first required here". You'll presumably need some mechanism to
determine the type being compared (something like
`FD->getParamDecl(0)->getType().getNonReferenceType().getUnqualifiedType()`,
though maybe you can share some code with `CheckExplicitlyDefaultedComparison`).
================
Comment at: clang/test/CXX/class/class.compare/class.compare.default/p6.cpp:1
+// RUN: %clang_cc1 -std=c++2a -verify %s
+
----------------
The file names here indicate the paragraph within the standard that's being
tested; p6 would be tests for paragraph 6 of [class.compare.default]. This test
doesn't appear to have anything to do with [class.compare.default] paragraph 6;
it appears to be testing paragraph 1. Please can you move these tests to p1.cpp
as appropriate?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103929/new/
https://reviews.llvm.org/D103929
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
