[cfe-dev] RFC: Defaulted comparison operators
Oleg Smolsky
oleg.smolsky at riverbed.com
Thu Dec 26 19:06:03 PST 2013
Hi all, I've started working on an ISO C++ proposal (attached) and would
like to ask for feedback on the prototype implementation (attached).
The point here is to extend the notion of "special member functions" to
include operator==() and its boolean inverse. The patch is minimal and
has a few "FIXME:" tags due to the lack of Clang knowledge...
Could you folks review and comment please?
Thanks in advance,
Oleg.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131226/63ef4784/attachment.html>
-------------- next part --------------
commit ff30be8902fdbb2397a471bc8dea13dbe4d84b59
Author: Oleg Smolsky <oleg at smolsky.net>
Date: Mon Dec 23 20:08:11 2013 -0800
First cut of (in)equality operators
- works for built-in and composite (user-defined) types
- minimal validity checks
ToDo:
- need to fix a few FIXME tags
diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h
index 75f20e2..f42b84c 100644
--- a/include/clang/AST/DeclCXX.h
+++ b/include/clang/AST/DeclCXX.h
@@ -1712,6 +1712,16 @@ public:
/// \brief Determine whether this is a move assignment operator.
bool isMoveAssignmentOperator() const;
+ /// \brief Determine whether this is an equality operator.
+ bool isEqualityOperator() const {
+ return getOverloadedOperator() == OO_EqualEqual && !isStatic();
+ }
+
+ /// \brief Determine whether this is an inequality operator.
+ bool isInequalityOperator() const {
+ return getOverloadedOperator() == OO_ExclaimEqual && !isStatic();
+ }
+
CXXMethodDecl *getCanonicalDecl() {
return cast<CXXMethodDecl>(FunctionDecl::getCanonicalDecl());
}
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 838ce63..09de6d2 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6111,6 +6111,8 @@ def err_out_of_line_default_deletes : Error<
"defaulting this %select{default constructor|copy constructor|move "
"constructor|copy assignment operator|move assignment operator|destructor}0 "
"would delete it after its first declaration">;
+def err_defaulted_comp_member_return_type : Error<
+ "explicitly-defaulted %0 operator must return %1">;
def warn_vbase_moved_multiple_times : Warning<
"defaulted move assignment operator of %0 will move assign virtual base "
"class %1 multiple times">, InGroup<DiagGroup<"multiple-move-vbase">>;
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 8533972..d0d8328 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -819,6 +819,8 @@ public:
CXXCopyAssignment,
CXXMoveAssignment,
CXXDestructor,
+ CXXEqualityOperator,
+ CXXInequalityOperator,
CXXInvalid
};
@@ -3953,6 +3955,14 @@ public:
void DefineImplicitCopyAssignment(SourceLocation CurrentLocation,
CXXMethodDecl *MethodDecl);
+ /// \brief Defines a default-declared equality operator.
+ void DefineDefaultEquality(SourceLocation CurrentLocation,
+ CXXMethodDecl *MethodDecl);
+
+ /// \brief Defines a default-declared inquality operator.
+ void DefineDefaultInequality(SourceLocation CurrentLocation,
+ CXXMethodDecl *MethodDecl);
+
/// \brief Declare the implicit move assignment operator for the given class.
///
/// \param ClassDecl The Class declaration into which the implicit
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 30ec0c4..ab1c995 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -2248,6 +2248,10 @@ Sema::CXXSpecialMember Sema::getSpecialMember(const CXXMethodDecl *MD) {
return Sema::CXXCopyAssignment;
} else if (MD->isMoveAssignmentOperator()) {
return Sema::CXXMoveAssignment;
+ } else if (MD->isEqualityOperator()) {
+ return Sema::CXXEqualityOperator;
+ } else if (MD->isInequalityOperator()) {
+ return Sema::CXXInequalityOperator;
}
return Sema::CXXInvalid;
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 117c37b..35dd28b 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -37,6 +37,7 @@
#include "clang/Sema/ParsedTemplate.h"
#include "clang/Sema/Scope.h"
#include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/SemaDiagnostic.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include <map>
@@ -4614,6 +4615,8 @@ static bool defaultedSpecialMemberIsConstexpr(Sema &S, CXXRecordDecl *ClassDecl,
Ctor = false;
break;
+ case Sema::CXXEqualityOperator:
+ case Sema::CXXInequalityOperator:
case Sema::CXXDestructor:
case Sema::CXXInvalid:
return false;
@@ -4694,6 +4697,11 @@ computeImplicitExceptionSpec(Sema &S, SourceLocation Loc, CXXMethodDecl *MD) {
return S.ComputeDefaultedMoveAssignmentExceptionSpec(MD);
case Sema::CXXDestructor:
return S.ComputeDefaultedDtorExceptionSpec(MD);
+ case Sema::CXXEqualityOperator:
+ case Sema::CXXInequalityOperator:
+ // FIXME:Oleg:
+ // - do I need to check whether we have built-ins vs overloaded calls?
+ return Sema::ImplicitExceptionSpecification(S);
case Sema::CXXInvalid:
break;
}
@@ -4791,7 +4799,10 @@ void Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD) {
CanHaveConstParam = RD->implicitCopyConstructorHasConstParam();
else if (CSM == CXXCopyAssignment)
CanHaveConstParam = RD->implicitCopyAssignmentHasConstParam();
+ else if (CSM == CXXEqualityOperator || CSM == CXXInequalityOperator)
+ CanHaveConstParam = true;
+ // Verify assignment operator declaration
QualType ReturnType = Context.VoidTy;
if (CSM == CXXCopyAssignment || CSM == CXXMoveAssignment) {
// Check for return type matching.
@@ -4812,6 +4823,25 @@ void Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD) {
}
}
+ // Verify equality operator declaration
+ if (CSM == CXXEqualityOperator || CSM == CXXInequalityOperator) {
+ // Check the return type
+ if (!Context.hasSameType(Type->getResultType(), Context.BoolTy)) {
+ Diag(MD->getLocation(), diag::err_defaulted_comp_member_return_type)
+ << "equality" << Context.BoolTy;
+ HadError = true;
+ }
+
+ // A defaulted special member needs to be "const"
+#if 0 // FIXME:Oleg:
+ if (Type->getTypeQuals()) {
+ Diag(MD->getLocation(), diag::err_defaulted_special_member_quals)
+ << true << getLangOpts().CPlusPlus1y;
+ HadError = true;
+ }
+#endif
+ }
+
// Check for parameter type matching.
QualType ArgType = ExpectedParams ? Type->getArgType(0) : QualType();
bool HasConstParam = false;
@@ -4832,12 +4862,15 @@ void Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD) {
diag::err_defaulted_special_member_copy_const_param)
<< (CSM == CXXCopyAssignment);
// FIXME: Explain why this special member can't be const.
+ HadError = true;
+ } else if (CSM == CXXEqualityOperator || CSM == CXXInequalityOperator) {
+ assert(false && "Const is OK on params, so this should never execute");
} else {
Diag(MD->getLocation(),
diag::err_defaulted_special_member_move_const_param)
<< (CSM == CXXMoveAssignment);
+ HadError = true;
}
- HadError = true;
}
} else if (ExpectedParams) {
// A copy assignment operator can take its argument by value, but a
@@ -4883,7 +4916,11 @@ void Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD) {
}
// If a function is explicitly defaulted on its first declaration,
- if (First) {
+ // FIXME:Oleg:
+ // - the point here is that the return type of default (in)equality members
+ // must not be mutated
+ // - yet, is it correct to boardly hack this out?
+ if (First && CSM != CXXEqualityOperator && CSM != CXXInequalityOperator) {
// -- it is implicitly considered to be constexpr if the implicit
// definition would be,
MD->setConstexpr(Constexpr);
@@ -4996,6 +5033,10 @@ struct SpecialMemberDeletionInfo {
IsAssignment = true;
IsMove = true;
break;
+ case Sema::CXXEqualityOperator:
+ case Sema::CXXInequalityOperator:
+ ConstArg = true;
+ break;
case Sema::CXXDestructor:
break;
case Sema::CXXInvalid:
@@ -5415,6 +5456,10 @@ static bool findTrivialSpecialMember(Sema &S, CXXRecordDecl *RD,
case Sema::CXXInvalid:
llvm_unreachable("not a special member");
+ case Sema::CXXEqualityOperator:
+ case Sema::CXXInequalityOperator:
+ return false;
+
case Sema::CXXDefaultConstructor:
// C++11 [class.ctor]p5:
// A default constructor is trivial if:
@@ -5714,6 +5759,10 @@ bool Sema::SpecialMemberIsTrivial(CXXMethodDecl *MD, CXXSpecialMember CSM,
break;
}
+ case CXXEqualityOperator:
+ case CXXInequalityOperator:
+ return false;
+
case CXXInvalid:
llvm_unreachable("not a special member");
}
@@ -9600,6 +9649,253 @@ void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation,
}
}
+static ExprResult
+buildSingleEqualityCheck(Sema &S, SourceLocation Loc, QualType T,
+ const ExprBuilder &LHS, const ExprBuilder &RHS) {
+ // Handle builtins first
+ if (T->isBuiltinType())
+ return S.CreateBuiltinBinOp(Loc, BO_EQ, LHS.build(S, Loc), RHS.build(S, Loc));
+
+ const RecordType *RecordTy = T->getAs<RecordType>();
+ CXXRecordDecl *ClassDecl = cast<CXXRecordDecl>(RecordTy->getDecl());
+
+ // Look for operator==()
+ DeclarationName Name = S.Context.DeclarationNames.getCXXOperatorName(OO_EqualEqual);
+ LookupResult OpLookup(S, Name, Loc, Sema::LookupOrdinaryName);
+ S.LookupQualifiedName(OpLookup, ClassDecl, false);
+
+ // Create the nested-name-specifier that will be used to qualify the
+ // reference to operator=; this is required to suppress the virtual
+ // call mechanism.
+ CXXScopeSpec SS;
+ const Type *CanonicalT = S.Context.getCanonicalType(T.getTypePtr());
+ SS.MakeTrivial(S.Context,
+ NestedNameSpecifier::Create(S.Context, 0, false,
+ CanonicalT),
+ Loc);
+
+ // Create the reference to the LHS' operator==() member function
+ ExprResult OpEqualRef
+ = S.BuildMemberReferenceExpr(LHS.build(S, Loc), T, Loc, /*isArrow=*/false,
+ SS, /*TemplateKWLoc=*/SourceLocation(),
+ /*FirstQualifierInScope=*/0,
+ OpLookup,
+ /*TemplateArgs=*/0,
+ /*SuppressQualifierCheck=*/true);
+ if (OpEqualRef.isInvalid())
+ return ExprError();
+
+ //
+ // Finally build the call to the equality operator
+ //
+ Expr *Arg = RHS.build(S, Loc);
+ ExprResult Call = S.BuildCallToMemberFunction(/*Scope=*/0,
+ OpEqualRef.takeAs<Expr>(),
+ Loc, Arg, Loc);
+ if (Call.isInvalid())
+ return ExprError();
+
+ return Call;
+}
+
+void Sema::DefineDefaultEquality(SourceLocation CurrentLocation,
+ CXXMethodDecl *EqualityOperator) {
+ assert((EqualityOperator->isDefaulted() &&
+ EqualityOperator->isOverloadedOperator() &&
+ EqualityOperator->getOverloadedOperator() == OO_EqualEqual
+ /* &&
+ !EqualityOperator->doesThisDeclarationHaveABody() &&
+ !EqualityOperator->isDeleted()*/) &&
+ "DefineDefaultEquality called for wrong function");
+
+ CXXRecordDecl *ClassDecl = EqualityOperator->getParent();
+
+ if (ClassDecl->isInvalidDecl() || EqualityOperator->isInvalidDecl()) {
+ EqualityOperator->setInvalidDecl();
+ return;
+ }
+
+ SynthesizedFunctionScope Scope(*this, EqualityOperator);
+ DiagnosticErrorTrap Trap(Diags);
+
+ // The statements that form the synthesized function body.
+ SmallVector<Stmt*, 8> Statements;
+
+ // The parameter for the "other" object, which we are copying from.
+ ParmVarDecl *Other = EqualityOperator->getParamDecl(0);
+ Qualifiers OtherQuals = Other->getType().getQualifiers();
+ QualType OtherRefType = Other->getType();
+ if (const LValueReferenceType *OtherRef
+ = OtherRefType->getAs<LValueReferenceType>()) {
+ OtherRefType = OtherRef->getPointeeType();
+ OtherQuals = OtherRefType.getQualifiers();
+ }
+
+ // Our location for everything implicitly-generated.
+ SourceLocation Loc = EqualityOperator->getLocation();
+
+ // Builds a DeclRefExpr for the "other" object.
+ RefBuilder OtherRef(Other, OtherRefType);
+
+ // Builds the "this" pointer.
+ ThisBuilder This;
+
+ // Compare each non-static member
+ bool Invalid = false;
+ for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin(),
+ FieldEnd = ClassDecl->field_end();
+ Field != FieldEnd; ++Field) {
+ if (Field->isUnnamedBitfield())
+ continue;
+
+ if (Field->isInvalidDecl()) {
+ Invalid = true;
+ continue;
+ }
+
+ // Suppress comparing zero-width bitfields.
+ if (Field->isBitField() && Field->getBitWidthValue(Context) == 0)
+ continue;
+
+ QualType FieldType = Field->getType().getNonReferenceType();
+ if (FieldType->isIncompleteArrayType()) {
+ assert(ClassDecl->hasFlexibleArrayMember() &&
+ "Incomplete array type is not valid");
+ continue;
+ }
+
+ // Build references to the field in the object we're copying from and to.
+ LookupResult MemberLookup(*this, Field->getDeclName(), Loc,
+ LookupMemberName);
+ MemberLookup.addDecl(*Field);
+ MemberLookup.resolveKind();
+
+ MemberBuilder RHS(OtherRef, OtherRefType, /*IsArrow=*/false, MemberLookup);
+ MemberBuilder LHS(This, getCurrentThisType(), /*IsArrow=*/true, MemberLookup);
+
+ // Build member comparison
+ ExprResult Compare = buildSingleEqualityCheck(*this, Loc, FieldType, LHS, RHS);
+ if (Compare.isInvalid()) {
+ EqualityOperator->setInvalidDecl();
+ return;
+ }
+
+ // The "return false" bit
+ ExprResult Val = ActOnIntegerConstant(Loc, 0);
+ StmtResult Return = ActOnReturnStmt(Loc, Val.get());
+
+ // The whole conditional expr
+ StmtResult If = Owned(new (Context) IfStmt(Context, Loc, 0, Compare.get(), Return.get()));
+ if (If.isInvalid()) {
+ Diag(CurrentLocation, diag::note_member_synthesized_at)
+ << CXXEqualityOperator << Context.getTagDeclType(ClassDecl);
+ EqualityOperator->setInvalidDecl();
+ return;
+ }
+
+ // Success! Record the single member's check
+ Statements.push_back(If.takeAs<Stmt>());
+ }
+
+ if (!Invalid) {
+ // Add a "return true;" to the end of the method
+ ExprResult Val = ActOnIntegerConstant(Loc, 1);
+ StmtResult Return = ActOnReturnStmt(Loc, Val.get());
+ if (Return.isInvalid()) {
+ Diag(CurrentLocation, diag::note_member_synthesized_at)
+ << CXXEqualityOperator << Context.getTagDeclType(ClassDecl);
+ EqualityOperator->setInvalidDecl();
+ return;
+ }
+ Statements.push_back(Return.takeAs<Stmt>());
+ }
+
+ if (Invalid) {
+ EqualityOperator->setInvalidDecl();
+ return;
+ }
+
+ StmtResult Body;
+ {
+ CompoundScopeRAII CompoundScope(*this);
+ Body = ActOnCompoundStmt(Loc, Loc, Statements,
+ /*isStmtExpr=*/false);
+ assert(!Body.isInvalid() && "Compound statement creation cannot fail");
+ }
+ EqualityOperator->setBody(Body.takeAs<Stmt>());
+
+ if (ASTMutationListener *L = getASTMutationListener()) {
+ L->CompletedImplicitDefinition(EqualityOperator);
+ }
+}
+
+void Sema::DefineDefaultInequality(SourceLocation CurrentLocation,
+ CXXMethodDecl *InequalityOperator) {
+ assert((InequalityOperator->isDefaulted() &&
+ InequalityOperator->isOverloadedOperator() &&
+ InequalityOperator->getOverloadedOperator() == OO_ExclaimEqual
+ /* &&
+ !InequalityOperator->doesThisDeclarationHaveABody() &&
+ !InequalityOperator->isDeleted()*/) &&
+ "DefineDefaultInequality called for wrong function");
+
+ CXXRecordDecl *ClassDecl = InequalityOperator->getParent();
+ QualType ClassType = Context.getTypeDeclType(ClassDecl);
+
+ if (ClassDecl->isInvalidDecl() || InequalityOperator->isInvalidDecl()) {
+ InequalityOperator->setInvalidDecl();
+ return;
+ }
+
+ SynthesizedFunctionScope Scope(*this, InequalityOperator);
+ DiagnosticErrorTrap Trap(Diags);
+
+ // The parameter for the "other" object, which we are copying from.
+ ParmVarDecl *Other = InequalityOperator->getParamDecl(0);
+ Qualifiers OtherQuals = Other->getType().getQualifiers();
+ QualType OtherRefType = Other->getType();
+ if (const LValueReferenceType *OtherRef
+ = OtherRefType->getAs<LValueReferenceType>()) {
+ OtherRefType = OtherRef->getPointeeType();
+ OtherQuals = OtherRefType.getQualifiers();
+ }
+
+ // Our location for everything implicitly-generated.
+ SourceLocation Loc = InequalityOperator->getLocation();
+
+ // Get "this" and "param" refs
+ RefBuilder OtherRef(Other, OtherRefType);
+ ThisBuilder This;
+
+ // Build the following expression (as I know it is a composite one):
+ // (*this).operator==(param)
+ ExprResult Compare = buildSingleEqualityCheck(*this, Loc, ClassType, DerefBuilder(This), OtherRef);
+ if (Compare.isInvalid()) {
+ InequalityOperator->setInvalidDecl();
+ return;
+ }
+
+ // Build the negation bit
+ ExprResult Negation = CreateBuiltinUnaryOp(Loc, UO_LNot, Compare.get());
+ if (Negation.isInvalid()) {
+ InequalityOperator->setInvalidDecl();
+ return;
+ }
+
+ // Build the return statement
+ StmtResult Return = ActOnReturnStmt(Loc, Negation.get());
+ if (Return.isInvalid()) {
+ InequalityOperator->setInvalidDecl();
+ return;
+ }
+
+ InequalityOperator->setBody(Return.takeAs<Stmt>());
+
+ if (ASTMutationListener *L = getASTMutationListener()) {
+ L->CompletedImplicitDefinition(InequalityOperator);
+ }
+}
+
Sema::ImplicitExceptionSpecification
Sema::ComputeDefaultedMoveAssignmentExceptionSpec(CXXMethodDecl *MD) {
CXXRecordDecl *ClassDecl = MD->getParent();
@@ -11995,8 +12291,15 @@ void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation DefaultLoc) {
// If the method was defaulted on its first declaration, we will have
// already performed the checking in CheckCompletedCXXClass. Such a
// declaration doesn't trigger an implicit definition.
- if (Primary == Primary->getCanonicalDecl())
+
+ // FIXME:Oleg:
+ // - I have no idea what this means, yet I need to generate the code
+ //
+ if (Primary == Primary->getCanonicalDecl() &&
+ /* hack */ Member != CXXEqualityOperator &&
+ Member != CXXInequalityOperator) {
return;
+ }
CheckExplicitlyDefaultedSpecialMember(MD);
@@ -12028,6 +12331,12 @@ void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation DefaultLoc) {
case CXXMoveAssignment:
DefineImplicitMoveAssignment(DefaultLoc, MD);
break;
+ case CXXEqualityOperator:
+ DefineDefaultEquality(DefaultLoc, MD);
+ break;
+ case CXXInequalityOperator:
+ DefineDefaultInequality(DefaultLoc, MD);
+ break;
case CXXInvalid:
llvm_unreachable("Invalid special member.");
}
More information about the cfe-dev
mailing list