From: Richard Smith Date: Mon, 9 Dec 2019 20:08:59 +0000 (-0800) Subject: [c++20] Fix incorrect assumptions in checks for comparison category types. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=56bba012d9729af8ff4252dc860f1f7696942f1a;p=platform%2Fupstream%2Fllvm.git [c++20] Fix incorrect assumptions in checks for comparison category types. In the presence of modules, we can have multiple lookup results for the same entity, and we need to re-check for completeness each time we consider a type. --- diff --git a/clang/lib/AST/ComparisonCategories.cpp b/clang/lib/AST/ComparisonCategories.cpp index 8999913..3fb500c5 100644 --- a/clang/lib/AST/ComparisonCategories.cpp +++ b/clang/lib/AST/ComparisonCategories.cpp @@ -59,7 +59,7 @@ ComparisonCategoryInfo::ValueInfo *ComparisonCategoryInfo::lookupValueInfo( // a new entry representing it. DeclContextLookupResult Lookup = Record->getCanonicalDecl()->lookup( &Ctx.Idents.get(ComparisonCategories::getResultString(ValueKind))); - if (Lookup.size() != 1 || !isa(Lookup.front())) + if (Lookup.empty() || !isa(Lookup.front())) return nullptr; Objects.emplace_back(ValueKind, cast(Lookup.front())); return &Objects.back(); @@ -70,7 +70,7 @@ static const NamespaceDecl *lookupStdNamespace(const ASTContext &Ctx, if (!StdNS) { DeclContextLookupResult Lookup = Ctx.getTranslationUnitDecl()->lookup(&Ctx.Idents.get("std")); - if (Lookup.size() == 1) + if (!Lookup.empty()) StdNS = dyn_cast(Lookup.front()); } return StdNS; @@ -81,7 +81,7 @@ static CXXRecordDecl *lookupCXXRecordDecl(const ASTContext &Ctx, ComparisonCategoryType Kind) { StringRef Name = ComparisonCategories::getCategoryString(Kind); DeclContextLookupResult Lookup = StdNS->lookup(&Ctx.Idents.get(Name)); - if (Lookup.size() == 1) + if (!Lookup.empty()) if (CXXRecordDecl *RD = dyn_cast(Lookup.front())) return RD; return nullptr; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 0a7c41c..28159b9c 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -7501,8 +7501,7 @@ public: VarDecl *EqualVD = S.Context.CompCategories.getInfoForType(StrongOrdering) .getValueInfo(ComparisonCategoryResult::Equal) ->VD; - RetVal = S.BuildDeclarationNameExpr( - CXXScopeSpec(), DeclarationNameInfo(), EqualVD); + RetVal = getDecl(EqualVD); if (RetVal.isInvalid()) return StmtError(); RetVal = buildStaticCastToR(RetVal.get()); @@ -7527,10 +7526,14 @@ public: } private: + ExprResult getDecl(ValueDecl *VD) { + return S.BuildDeclarationNameExpr( + CXXScopeSpec(), DeclarationNameInfo(VD->getDeclName(), Loc), VD); + } + ExprResult getParam(unsigned I) { ParmVarDecl *PD = FD->getParamDecl(I); - return S.BuildDeclarationNameExpr( - CXXScopeSpec(), DeclarationNameInfo(PD->getDeclName(), Loc), PD); + return getDecl(PD); } ExprPair getCompleteObject() { @@ -7622,8 +7625,7 @@ private: Stmt *InitStmt = new (S.Context) DeclStmt(DeclGroupRef(VD), Loc, Loc); // cmp != 0 - ExprResult VDRef = S.BuildDeclarationNameExpr( - CXXScopeSpec(), DeclarationNameInfo(Name, Loc), VD); + ExprResult VDRef = getDecl(VD); if (VDRef.isInvalid()) return StmtError(); llvm::APInt ZeroVal(S.Context.getIntWidth(S.Context.IntTy), 0); @@ -7639,8 +7641,7 @@ private: return StmtError(); // return cmp; - VDRef = S.BuildDeclarationNameExpr( - CXXScopeSpec(), DeclarationNameInfo(Name, Loc), VD); + VDRef = getDecl(VD); if (VDRef.isInvalid()) return StmtError(); StmtResult ReturnStmt = S.BuildReturnStmt(Loc, VDRef.get()); @@ -10235,11 +10236,25 @@ QualType Sema::CheckComparisonCategoryType(ComparisonCategoryType Kind, assert(getLangOpts().CPlusPlus && "Looking for comparison category type outside of C++."); + // Use an elaborated type for diagnostics which has a name containing the + // prepended 'std' namespace but not any inline namespace names. + auto TyForDiags = [&](ComparisonCategoryInfo *Info) { + auto *NNS = + NestedNameSpecifier::Create(Context, nullptr, getStdNamespace()); + return Context.getElaboratedType(ETK_None, NNS, Info->getType()); + }; + // Check if we've already successfully checked the comparison category type // before. If so, skip checking it again. ComparisonCategoryInfo *Info = Context.CompCategories.lookupInfo(Kind); - if (Info && FullyCheckedComparisonCategories[static_cast(Kind)]) + if (Info && FullyCheckedComparisonCategories[static_cast(Kind)]) { + // The only thing we need to check is that the type has a reachable + // definition in the current context. + if (RequireCompleteType(Loc, TyForDiags(Info), diag::err_incomplete_type)) + return QualType(); + return Info->getType(); + } // If lookup failed if (!Info) { @@ -10258,18 +10273,10 @@ QualType Sema::CheckComparisonCategoryType(ComparisonCategoryType Kind, if (Info->Record->hasDefinition()) Info->Record = Info->Record->getDefinition(); - // Use an elaborated type for diagnostics which has a name containing the - // prepended 'std' namespace but not any inline namespace names. - QualType TyForDiags = [&]() { - auto *NNS = - NestedNameSpecifier::Create(Context, nullptr, getStdNamespace()); - return Context.getElaboratedType(ETK_None, NNS, Info->getType()); - }(); - - if (RequireCompleteType(Loc, TyForDiags, diag::err_incomplete_type)) + if (RequireCompleteType(Loc, TyForDiags(Info), diag::err_incomplete_type)) return QualType(); - InvalidSTLDiagnoser UnsupportedSTLError{*this, Loc, TyForDiags}; + InvalidSTLDiagnoser UnsupportedSTLError{*this, Loc, TyForDiags(Info)}; if (!Info->Record->isTriviallyCopyable()) return UnsupportedSTLError(USS_NonTrivial); diff --git a/clang/test/SemaCXX/compare-cxx2a.cpp b/clang/test/SemaCXX/compare-cxx2a.cpp index 1aca2dd..da834e9 100644 --- a/clang/test/SemaCXX/compare-cxx2a.cpp +++ b/clang/test/SemaCXX/compare-cxx2a.cpp @@ -3,6 +3,9 @@ // RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -fsyntax-only -pedantic -verify -Wsign-compare -std=c++2a %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -std=c++2a -x c++ %S/Inputs/std-compare.h -emit-pch -o %t.pch +// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -fsyntax-only -pedantic -verify -Wsign-compare -std=c++2a %s -include-pch %t.pch + #include "Inputs/std-compare.h" #define ASSERT_TYPE(...) static_assert(__is_same(__VA_ARGS__)) diff --git a/clang/test/SemaCXX/compare-modules-cxx2a.cpp b/clang/test/SemaCXX/compare-modules-cxx2a.cpp new file mode 100644 index 0000000..fa91800 --- /dev/null +++ b/clang/test/SemaCXX/compare-modules-cxx2a.cpp @@ -0,0 +1,54 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery + +#pragma clang module build compare +module compare { + explicit module cmp {} + explicit module other {} +} +#pragma clang module contents +#pragma clang module begin compare.cmp +#include "std-compare.h" +#pragma clang module end +#pragma clang module endbuild + +struct CC { CC(...); }; + +void a() { void(0 <=> 0); } // expected-error {{include }} + +struct A { + CC operator<=>(const A&) const = default; // expected-error {{include }} +}; +auto va = A() <=> A(); // expected-note {{required here}} + +#pragma clang module import compare.other + +// expected-note@std-compare.h:* 2+{{previous definition}} + +void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported}} + +struct B { + CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported}} +}; +auto vb = B() <=> B(); // expected-note {{required here}} + +#pragma clang module import compare.cmp + +void c() { void(0 <=> 0); } + +struct C { + CC operator<=>(const C&) const = default; +}; +auto vc = C() <=> C(); + + +#pragma clang module build compare2 +module compare2 {} +#pragma clang module contents +#pragma clang module begin compare2 +#include "std-compare.h" +#pragma clang module end +#pragma clang module endbuild + +#pragma clang module import compare2 + +void g() { void(0.0 <=> 0.0); }