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@+1 {{cannot deduce return type of 'operator<=>' because type partial_ordering was not found; include <compare>}} + // expected-error@+1 {{cannot deduce return type of 'operator<=>' because type 'std::partial_ordering' was not found; include <compare>}} (void)(0.0 <=> 42.123); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits