r331707 - [C++2a] Implement operator<=>: Address bugs and post-commit review comments after r331677.
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Mon May 7 17:52:19 PDT 2018
Author: ericwf
Date: Mon May 7 17:52:19 2018
New Revision: 331707
URL: http://llvm.org/viewvc/llvm-project?rev=331707&view=rev
Log:
[C++2a] Implement operator<=>: Address bugs and post-commit review comments after r331677.
This patch addresses some mostly trivial post-commit review comments received
on r331677.
Additionally, this patch fixes an assertion in `getNarrowingKind` caused by
the use of an uninitialized value from `checkThreeWayNarrowingConversion`.
Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/AST/ComparisonCategories.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ComparisonCategories.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaCXX/compare-cxx2a.cpp
cfe/trunk/test/SemaCXX/std-compare-cxx2a.cpp
Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=331707&r1=331706&r2=331707&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Mon May 7 17:52:19 2018
@@ -1979,8 +1979,8 @@ public:
QualType GetBuiltinType(unsigned ID, GetBuiltinTypeError &Error,
unsigned *IntegerConstantArgs = nullptr) const;
- /// \brief Types and expressions required to build C++2a three-way comparisons
- /// using operator<=>, including the values return by builtin <=> operators.
+ /// Types and expressions required to build C++2a three-way comparisons
+ /// using operator<=>, including the values return by builtin <=> operators.
ComparisonCategories CompCategories;
private:
Modified: cfe/trunk/include/clang/AST/ComparisonCategories.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ComparisonCategories.h?rev=331707&r1=331706&r2=331707&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ComparisonCategories.h (original)
+++ cfe/trunk/include/clang/AST/ComparisonCategories.h Mon May 7 17:52:19 2018
@@ -35,7 +35,7 @@ class Sema;
class QualType;
class NamespaceDecl;
-/// \brief An enumeration representing the different comparison categories
+/// An enumeration representing the different comparison categories
/// types.
///
/// C++2a [cmp.categories.pre] The types weak_equality, strong_equality,
@@ -51,9 +51,9 @@ enum class ComparisonCategoryType : unsi
Last = StrongOrdering
};
-/// \brief An enumeration representing the possible results of a three-way
-/// comparison. These values map onto instances of comparison category types
-/// defined in the standard library. i.e. 'std::strong_ordering::less'.
+/// An enumeration representing the possible results of a three-way
+/// comparison. These values map onto instances of comparison category types
+/// defined in the standard library. e.g. 'std::strong_ordering::less'.
enum class ComparisonCategoryResult : unsigned char {
Equal,
Equivalent,
@@ -79,39 +79,26 @@ public:
VarDecl *VD;
ValueInfo(ComparisonCategoryResult Kind, VarDecl *VD)
- : Kind(Kind), VD(VD), HasValue(false) {}
+ : Kind(Kind), VD(VD) {}
- /// \brief True iff we've successfully evaluated the variable as a constant
+ /// True iff we've successfully evaluated the variable as a constant
/// expression and extracted its integer value.
- bool hasValidIntValue() const { return HasValue; }
+ bool hasValidIntValue() const;
- /// \brief Get the constant integer value used by this variable to represent
+ /// Get the constant integer value used by this variable to represent
/// the comparison category result type.
- llvm::APSInt getIntValue() const {
- assert(hasValidIntValue());
- return IntValue;
- }
-
- void setIntValue(llvm::APSInt Val) {
- IntValue = Val;
- HasValue = true;
- }
-
- private:
- friend class ComparisonCategoryInfo;
- llvm::APSInt IntValue;
- bool HasValue : 1;
+ llvm::APSInt getIntValue() const;
};
private:
const ASTContext &Ctx;
- /// \brief A map containing the comparison category result decls from the
+ /// A map containing the comparison category result decls from the
/// standard library. The key is a value of ComparisonCategoryResult.
mutable llvm::SmallVector<
ValueInfo, static_cast<unsigned>(ComparisonCategoryResult::Last) + 1>
Objects;
- /// \brief Lookup the ValueInfo struct for the specified ValueKind. If the
+ /// Lookup the ValueInfo struct for the specified ValueKind. If the
/// VarDecl for the value cannot be found, nullptr is returned.
///
/// If the ValueInfo does not have a valid integer value the variable
@@ -119,12 +106,12 @@ private:
ValueInfo *lookupValueInfo(ComparisonCategoryResult ValueKind) const;
public:
- /// \brief The declaration for the comparison category type from the
+ /// The declaration for the comparison category type from the
/// standard library.
// FIXME: Make this const
CXXRecordDecl *Record = nullptr;
- /// \brief The Kind of the comparison category type
+ /// The Kind of the comparison category type
ComparisonCategoryType Kind;
public:
@@ -139,7 +126,7 @@ public:
return Info;
}
- /// \brief True iff the comparison category is an equality comparison.
+ /// True iff the comparison category is an equality comparison.
bool isEquality() const { return !isOrdered(); }
/// \brief True iff the comparison category is a relational comparison.
@@ -149,22 +136,22 @@ public:
Kind == CCK::StrongOrdering;
}
- /// \brief True iff the comparison is "strong". i.e. it checks equality and
- /// not equivalence.
+ /// True iff the comparison is "strong". i.e. it checks equality and
+ /// not equivalence.
bool isStrong() const {
using CCK = ComparisonCategoryType;
return Kind == CCK::StrongEquality || Kind == CCK::StrongOrdering;
}
- /// \brief True iff the comparison is not totally ordered.
+ /// True iff the comparison is not totally ordered.
bool isPartial() const {
using CCK = ComparisonCategoryType;
return Kind == CCK::PartialOrdering;
}
- /// \brief Converts the specified result kind into the the correct result kind
- /// for this category. Specifically it lowers strong equality results to
- /// weak equivalence if needed.
+ /// Converts the specified result kind into the the correct result kind
+ /// for this category. Specifically it lowers strong equality results to
+ /// weak equivalence if needed.
ComparisonCategoryResult makeWeakResult(ComparisonCategoryResult Res) const {
using CCR = ComparisonCategoryResult;
if (!isStrong()) {
@@ -202,13 +189,13 @@ public:
static StringRef getCategoryString(ComparisonCategoryType Kind);
static StringRef getResultString(ComparisonCategoryResult Kind);
- /// \brief Return the list of results which are valid for the specified
- /// comparison category type.
+ /// Return the list of results which are valid for the specified
+ /// comparison category type.
static std::vector<ComparisonCategoryResult>
getPossibleResultsForType(ComparisonCategoryType Type);
- /// \brief Return the comparison category information for the category
- /// specified by 'Kind'.
+ /// Return the comparison category information for the category
+ /// specified by 'Kind'.
const ComparisonCategoryInfo &getInfo(ComparisonCategoryType Kind) const {
const ComparisonCategoryInfo *Result = lookupInfo(Kind);
assert(Result != nullptr &&
@@ -216,17 +203,18 @@ public:
return *Result;
}
- /// \brief Return the comparison category information as specified by
- /// `getCategoryForType(Ty)`. If the information is not already cached,
- /// the declaration is looked up and a cache entry is created.
- /// NOTE: Lookup is expected to succeed. Use lookupInfo if failure is possible.
+ /// Return the comparison category information as specified by
+ /// `getCategoryForType(Ty)`. If the information is not already cached,
+ /// the declaration is looked up and a cache entry is created.
+ /// NOTE: Lookup is expected to succeed. Use lookupInfo if failure is
+ /// possible.
const ComparisonCategoryInfo &getInfoForType(QualType Ty) const;
public:
- /// \brief Return the cached comparison category information for the
- /// specified 'Kind'. If no cache entry is present the comparison category
- /// type is looked up. If lookup fails nullptr is returned. Otherwise, a
- /// new cache entry is created and returned
+ /// Return the cached comparison category information for the
+ /// specified 'Kind'. If no cache entry is present the comparison category
+ /// type is looked up. If lookup fails nullptr is returned. Otherwise, a
+ /// new cache entry is created and returned
const ComparisonCategoryInfo *lookupInfo(ComparisonCategoryType Kind) const;
ComparisonCategoryInfo *lookupInfo(ComparisonCategoryType Kind) {
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=331707&r1=331706&r2=331707&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon May 7 17:52:19 2018
@@ -9426,7 +9426,7 @@ def err_multiversion_not_supported : Err
// three-way comparison operator diagnostics
def err_implied_comparison_category_type_not_found : Error<
- "cannot deduce return type of 'operator<=>' because type %0 was not found; "
+ "cannot deduce return type of 'operator<=>' because type '%0' was not found; "
"include <compare>">;
def err_spaceship_argument_narrowing : Error<
"argument to 'operator<=>' "
Modified: cfe/trunk/lib/AST/ComparisonCategories.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ComparisonCategories.cpp?rev=331707&r1=331706&r2=331707&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ComparisonCategories.cpp (original)
+++ cfe/trunk/lib/AST/ComparisonCategories.cpp Mon May 7 17:52:19 2018
@@ -20,34 +20,32 @@
using namespace clang;
-/// Attempt to determine the integer value used to represent the comparison
-/// category result by evaluating the initializer for the specified VarDecl as
-/// a constant expression and retreiving the value of the classes first
-/// (and only) field.
-///
-/// Note: The STL types are expected to have the form:
-/// struct X { T value; };
-/// where T is an integral or enumeration type.
-static bool evaluateIntValue(const ASTContext &Ctx,
- ComparisonCategoryInfo::ValueInfo *Info) {
- if (Info->hasValidIntValue())
+bool ComparisonCategoryInfo::ValueInfo::hasValidIntValue() const {
+ assert(VD && "must have var decl");
+ if (!VD->checkInitIsICE())
return false;
// Before we attempt to get the value of the first field, ensure that we
// actually have one (and only one) field.
- auto *Record = Info->VD->getType()->getAsCXXRecordDecl();
+ auto *Record = VD->getType()->getAsCXXRecordDecl();
if (std::distance(Record->field_begin(), Record->field_end()) != 1 ||
!Record->field_begin()->getType()->isIntegralOrEnumerationType())
- return true;
+ return false;
+
+ return true;
+}
- Expr::EvalResult Result;
- if (!Info->VD->hasInit() ||
- !Info->VD->getInit()->EvaluateAsRValue(Result, Ctx))
- return true;
-
- assert(Result.Val.isStruct());
- Info->setIntValue(Result.Val.getStructField(0).getInt());
- return false;
+/// Attempt to determine the integer value used to represent the comparison
+/// category result by evaluating the initializer for the specified VarDecl as
+/// a constant expression and retreiving the value of the class's first
+/// (and only) field.
+///
+/// Note: The STL types are expected to have the form:
+/// struct X { T value; };
+/// where T is an integral or enumeration type.
+llvm::APSInt ComparisonCategoryInfo::ValueInfo::getIntValue() const {
+ assert(hasValidIntValue() && "must have a valid value");
+ return VD->evaluateValue()->getStructField(0).getInt();
}
ComparisonCategoryInfo::ValueInfo *ComparisonCategoryInfo::lookupValueInfo(
@@ -55,24 +53,17 @@ ComparisonCategoryInfo::ValueInfo *Compa
// Check if we already have a cache entry for this value.
auto It = llvm::find_if(
Objects, [&](ValueInfo const &Info) { return Info.Kind == ValueKind; });
+ if (It != Objects.end())
+ return &(*It);
// We don't have a cached result. Lookup the variable declaration and create
// a new entry representing it.
- if (It == Objects.end()) {
- DeclContextLookupResult Lookup = Record->getCanonicalDecl()->lookup(
- &Ctx.Idents.get(ComparisonCategories::getResultString(ValueKind)));
- if (Lookup.size() != 1 || !isa<VarDecl>(Lookup.front()))
- return nullptr;
- Objects.emplace_back(ValueKind, cast<VarDecl>(Lookup.front()));
- It = Objects.end() - 1;
- }
- assert(It != Objects.end());
- // Success! Attempt to update the int value in case the variables initializer
- // wasn't present the last time we were here.
- ValueInfo *Info = &(*It);
- evaluateIntValue(Ctx, Info);
-
- return Info;
+ DeclContextLookupResult Lookup = Record->getCanonicalDecl()->lookup(
+ &Ctx.Idents.get(ComparisonCategories::getResultString(ValueKind)));
+ if (Lookup.size() != 1 || !isa<VarDecl>(Lookup.front()))
+ return nullptr;
+ Objects.emplace_back(ValueKind, cast<VarDecl>(Lookup.front()));
+ return &Objects.back();
}
static const NamespaceDecl *lookupStdNamespace(const ASTContext &Ctx,
Modified: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprAgg.cpp?rev=331707&r1=331706&r2=331707&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprAgg.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp Mon May 7 17:52:19 2018
@@ -956,7 +956,7 @@ void AggExprEmitter::VisitBinCmp(const B
if (!ArgTy->isIntegralOrEnumerationType() && !ArgTy->isRealFloatingType() &&
!ArgTy->isNullPtrType() && !ArgTy->isPointerType() &&
!ArgTy->isMemberPointerType() && !ArgTy->isAnyComplexType()) {
- return CGF.ErrorUnsupported(E, "aggregate three-way comparisoaoeun");
+ return CGF.ErrorUnsupported(E, "aggregate three-way comparison");
}
bool IsComplex = ArgTy->isAnyComplexType();
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=331707&r1=331706&r2=331707&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon May 7 17:52:19 2018
@@ -8938,8 +8938,10 @@ QualType Sema::CheckComparisonCategoryTy
// If lookup failed
if (!Info) {
+ auto NameForDiags =
+ llvm::Twine("std::") + ComparisonCategories::getCategoryString(Kind);
Diag(Loc, diag::err_implied_comparison_category_type_not_found)
- << ComparisonCategories::getCategoryString(Kind);
+ << NameForDiags.str();
return QualType();
}
@@ -8947,7 +8949,7 @@ QualType Sema::CheckComparisonCategoryTy
assert(Info->Record);
// Update the Record decl in case we encountered a forward declaration on our
- // first pass. FIXME(EricWF): This is a bit of a hack.
+ // first pass. FIXME: This is a bit of a hack.
if (Info->Record->hasDefinition())
Info->Record = Info->Record->getDefinition();
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=331707&r1=331706&r2=331707&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon May 7 17:52:19 2018
@@ -9757,12 +9757,12 @@ static bool checkThreeWayNarrowingConver
SourceLocation Loc) {
// Check for a narrowing implicit conversion.
StandardConversionSequence SCS;
+ SCS.setAsIdentityConversion();
SCS.setToType(0, FromType);
SCS.setToType(1, ToType);
- if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E)) {
- auto CastK = ICE->getCastKind();
- SCS.Second = castKindToImplicitConversionKind(CastK);
- }
+ if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E))
+ SCS.Second = castKindToImplicitConversionKind(ICE->getCastKind());
+
APValue PreNarrowingValue;
QualType PreNarrowingType;
switch (SCS.getNarrowingKind(S.Context, E, PreNarrowingValue,
Modified: cfe/trunk/test/SemaCXX/compare-cxx2a.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/compare-cxx2a.cpp?rev=331707&r1=331706&r2=331707&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/compare-cxx2a.cpp (original)
+++ cfe/trunk/test/SemaCXX/compare-cxx2a.cpp Mon May 7 17:52:19 2018
@@ -37,7 +37,6 @@ void test0(long a, unsigned long b) {
(void)((short) a <=> (unsigned short) b);
(void)((signed char) a <=> (unsigned char) b);
- (void)(A < 42);
// (A,b)
(void)(A <=> (unsigned long) b);
(void)(A <=> (unsigned int) b);
Modified: cfe/trunk/test/SemaCXX/std-compare-cxx2a.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/std-compare-cxx2a.cpp?rev=331707&r1=331706&r2=331707&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/std-compare-cxx2a.cpp (original)
+++ cfe/trunk/test/SemaCXX/std-compare-cxx2a.cpp Mon May 7 17:52:19 2018
@@ -3,7 +3,7 @@
// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -fsyntax-only -pedantic -verify -Wsign-compare -std=c++2a %s
void compare_not_found_test() {
- // expected-error at +1 {{cannot deduce return type of 'operator<=>' because type partial_ordering was not found; include <compare>}}
+ // expected-error at +1 {{cannot deduce return type of 'operator<=>' because type 'std::partial_ordering' was not found; include <compare>}}
(void)(0.0 <=> 42.123);
}
More information about the cfe-commits
mailing list