[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.

2 files changed, 29 insertions, 22 deletions

diff --git a/clang/lib/AST/ComparisonCategories.cpp b/clang/lib/AST/ComparisonCategories.cpp
index 8999913b728..3fb500c580e 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<VarDecl>(Lookup.front()))
+  if (Lookup.empty() || !isa<VarDecl>(Lookup.front()))
     return nullptr;
   Objects.emplace_back(ValueKind, cast<VarDecl>(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<NamespaceDecl>(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<CXXRecordDecl>(Lookup.front()))
       return RD;
   return nullptr;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 0a7c41c1eda..28159b9c081 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<unsigned>(Kind)])
+  if (Info && FullyCheckedComparisonCategories[static_cast<unsigned>(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);