[clang] diagnose invalid member pointer class on instantiation

This moves the diagnostic for member pointers pointing into
non-class into BuildMemberPointer, so that it can be
used from RebuildMemberPointer, when instantiating templates.

Also adds a minor tweak to the diagnostic when the member pointer is anonymous,
which was previously untested.

Also reverts #132501, which
disabled a failing test due to the regression which is now fixed.

No changelog, since this fixes a regression which has not been released
yet.

Fixes #132494

Author: mizvekov

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6976,7 +6976,7 @@ def err_illegal_decl_mempointer_to_reference : Error<
 def err_illegal_decl_mempointer_to_void : Error<
   "'%0' declared as a member pointer to void">;
 def err_illegal_decl_mempointer_in_nonclass
-    : Error<"'%0' does not point into a class">;
+    : Error<"%0 does not point into a class">;
 def err_reference_to_void : Error<"cannot form a reference to 'void'">;
 def err_nonfunction_block_type : Error<
   "block pointer to non-function type is invalid">;

clang/include/clang/Sema/Sema.h

@@ -14885,7 +14885,7 @@ class Sema final : public SemaBase {
   ///
   /// \returns a member pointer type, if successful, or a NULL type if there was
   /// an error.
-  QualType BuildMemberPointerType(QualType T, NestedNameSpecifier *Qualifier,
+  QualType BuildMemberPointerType(QualType T, const CXXScopeSpec &SS,
                                   CXXRecordDecl *Cls, SourceLocation Loc,
                                   DeclarationName Entity);
 

clang/lib/Sema/SemaType.cpp

@@ -2685,10 +2685,23 @@ QualType Sema::BuildFunctionType(QualType T,
   return Context.getFunctionType(T, ParamTypes, EPI);
 }
 
-QualType Sema::BuildMemberPointerType(QualType T,
-                                      NestedNameSpecifier *Qualifier,
+QualType Sema::BuildMemberPointerType(QualType T, const CXXScopeSpec &SS,
                                       CXXRecordDecl *Cls, SourceLocation Loc,
                                       DeclarationName Entity) {
+  if (!Cls && !isDependentScopeSpecifier(SS)) {
+    Cls = dyn_cast_or_null<CXXRecordDecl>(computeDeclContext(SS));
+    if (!Cls) {
+      auto D =
+          Diag(SS.getBeginLoc(), diag::err_illegal_decl_mempointer_in_nonclass)
+          << SS.getRange();
+      if (const IdentifierInfo *II = Entity.getAsIdentifierInfo())
+        D << II;
+      else
+        D << "member pointer";
+      return QualType();
+    }
+  }
+
   // Verify that we're not building a pointer to pointer to function with
   // exception specification.
   if (CheckDistantExceptionSpec(T)) {
@@ -2730,7 +2743,7 @@ QualType Sema::BuildMemberPointerType(QualType T,
   if (T->isFunctionType())
     adjustMemberFunctionCC(T, /*HasThisPointer=*/true, IsCtorOrDtor, Loc);
 
-  return Context.getMemberPointerType(T, Qualifier, Cls);
+  return Context.getMemberPointerType(T, SS.getScopeRep(), Cls);
 }
 
 QualType Sema::BuildBlockPointerType(QualType T,
@@ -5344,20 +5357,9 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
         // Avoid emitting extra errors if we already errored on the scope.
         D.setInvalidType(true);
         AreDeclaratorChunksValid = false;
-      } else if (auto *RD =
-                     dyn_cast_or_null<CXXRecordDecl>(S.computeDeclContext(SS));
-                 RD || S.isDependentScopeSpecifier(SS)) {
-        T = S.BuildMemberPointerType(T, SS.getScopeRep(), RD, DeclType.Loc,
-                                     D.getIdentifier());
       } else {
-        S.Diag(DeclType.Mem.Scope().getBeginLoc(),
-             diag::err_illegal_decl_mempointer_in_nonclass)
-          << (D.getIdentifier() ? D.getIdentifier()->getName() : "type name")
-          << DeclType.Mem.Scope().getRange();
-        D.setInvalidType(true);
-        AreDeclaratorChunksValid = false;
-        // FIXME: Maybe we could model these as as a MemberPointerType with a
-        // non-dependent, non-class qualifier anyway.
+        T = S.BuildMemberPointerType(T, SS, /*Cls=*/nullptr, DeclType.Loc,
+                                     D.getIdentifier());
       }
 
       if (T.isNull()) {
@@ -9255,10 +9257,10 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
   //         "Can't ask whether a dependent type is complete");
 
   if (const MemberPointerType *MPTy = T->getAs<MemberPointerType>()) {
-    if (!MPTy->getQualifier()->isDependent()) {
-      QualType T = Context.getTypeDeclType(MPTy->getMostRecentCXXRecordDecl());
-      if (getLangOpts().CompleteMemberPointers &&
-          !MPTy->getMostRecentCXXRecordDecl()->isBeingDefined() &&
+    if (CXXRecordDecl *RD = MPTy->getMostRecentCXXRecordDecl();
+        RD && !RD->isDependentType()) {
+      QualType T = Context.getTypeDeclType(RD);
+      if (getLangOpts().CompleteMemberPointers && !RD->isBeingDefined() &&
           RequireCompleteType(Loc, T, Kind, diag::err_memptr_incomplete))
         return true;
 

clang/lib/Sema/TreeTransform.h

@@ -864,8 +864,8 @@ class TreeTransform {
   /// By default, performs semantic analysis when building the member pointer
   /// type. Subclasses may override this routine to provide different behavior.
   QualType RebuildMemberPointerType(QualType PointeeType,
-                                    NestedNameSpecifier *Qualifier,
-                                    CXXRecordDecl *Cls, SourceLocation Sigil);
+                                    const CXXScopeSpec &SS, CXXRecordDecl *Cls,
+                                    SourceLocation Sigil);
 
   QualType RebuildObjCTypeParamType(const ObjCTypeParamDecl *Decl,
                                     SourceLocation ProtocolLAngleLoc,
@@ -5631,9 +5631,10 @@ TreeTransform<Derived>::TransformMemberPointerType(TypeLocBuilder &TLB,
       NewQualifierLoc.getNestedNameSpecifier() !=
           OldQualifierLoc.getNestedNameSpecifier() ||
       NewCls != OldCls) {
-    Result = getDerived().RebuildMemberPointerType(
-        PointeeType, NewQualifierLoc.getNestedNameSpecifier(), NewCls,
-        TL.getStarLoc());
+    CXXScopeSpec SS;
+    SS.Adopt(NewQualifierLoc);
+    Result = getDerived().RebuildMemberPointerType(PointeeType, SS, NewCls,
+                                                   TL.getStarLoc());
     if (Result.isNull())
       return QualType();
   }
@@ -17044,9 +17045,9 @@ TreeTransform<Derived>::RebuildReferenceType(QualType ReferentType,
 
 template <typename Derived>
 QualType TreeTransform<Derived>::RebuildMemberPointerType(
-    QualType PointeeType, NestedNameSpecifier *Qualifier, CXXRecordDecl *Cls,
+    QualType PointeeType, const CXXScopeSpec &SS, CXXRecordDecl *Cls,
     SourceLocation Sigil) {
-  return SemaRef.BuildMemberPointerType(PointeeType, Qualifier, Cls, Sigil,
+  return SemaRef.BuildMemberPointerType(PointeeType, SS, Cls, Sigil,
                                         getDerived().getBaseEntity());
 }
 

clang/test/SemaCXX/member-pointer.cpp

@@ -333,3 +333,14 @@ namespace test9 {
   struct C { int BAR::*mp; };
   // expected-error@-1 {{'BAR' is not a class, namespace, or enumeration}}
 } // namespace test9
+
+namespace GH132494 {
+  enum E {};
+
+  void f(int E::*); // expected-error {{member pointer does not point into a class}}
+
+  template <class T> struct A {
+    int T::*foo; // expected-error {{'foo' does not point into a class}}
+  };
+  template struct A<E>; // expected-note {{requested here}}
+} // namespace GH132494

libc/test/src/__support/CPP/type_traits_test.cpp

@@ -334,9 +334,7 @@ TEST(LlvmLibcTypeTraitsTest, is_class) {
   // Neither other types.
   EXPECT_FALSE((is_class_v<Union>));
   EXPECT_FALSE((is_class_v<int>));
-  // TODO: Re-enable the test after
-  // https://github.com/llvm/llvm-project/issues/132494 is fixed.
-  // EXPECT_FALSE((is_class_v<EnumClass>));
+  EXPECT_FALSE((is_class_v<EnumClass>));
 }
 
 TYPED_TEST(LlvmLibcTypeTraitsTest, is_const, UnqualObjectTypes) {