[clang] 25b3370 - PR52139: Properly handle more kinds of declaration when checking for

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 11 16:37:46 PDT 2021


Author: Richard Smith
Date: 2021-10-11T16:37:39-07:00
New Revision: 25b3370ff25f7a21f71e0dc6c4d7624d52cab604

URL: https://github.com/llvm/llvm-project/commit/25b3370ff25f7a21f71e0dc6c4d7624d52cab604
DIFF: https://github.com/llvm/llvm-project/commit/25b3370ff25f7a21f71e0dc6c4d7624d52cab604.diff

LOG: PR52139: Properly handle more kinds of declaration when checking for
usage of an abstract class type within itself.

We were missing handling for deduction guides (which would assert),
friend declarations, and variable templates. We were mishandling inline
variables and other variables defined inside the class definition.

These diagnostics should be downgraded to warnings, or perhaps removed
entirely, once we implement P0929R2.

Added: 
    

Modified: 
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/test/SemaCXX/abstract.cpp
    clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index b7f84eab2f7ef..31eda99089295 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5884,6 +5884,7 @@ struct CheckAbstractUsage {
     if (CT != Info.AbstractType) return;
 
     // It matched; do some magic.
+    // FIXME: These should be at most warnings. See P0929R2, CWG1640, CWG1646.
     if (Sel == Sema::AbstractArrayType) {
       Info.S.Diag(Ctx->getLocation(), diag::err_array_of_abstract_type)
         << T << TL.getSourceRange();
@@ -5902,19 +5903,31 @@ void AbstractUsageInfo::CheckType(const NamedDecl *D, TypeLoc TL,
 
 }
 
-/// Check for invalid uses of an abstract type in a method declaration.
+/// Check for invalid uses of an abstract type in a function declaration.
 static void CheckAbstractClassUsage(AbstractUsageInfo &Info,
-                                    CXXMethodDecl *MD) {
+                                    FunctionDecl *FD) {
   // No need to do the check on definitions, which require that
   // the return/param types be complete.
-  if (MD->doesThisDeclarationHaveABody())
+  if (FD->doesThisDeclarationHaveABody())
     return;
 
   // For safety's sake, just ignore it if we don't have type source
   // information.  This should never happen for non-implicit methods,
   // but...
-  if (TypeSourceInfo *TSI = MD->getTypeSourceInfo())
-    Info.CheckType(MD, TSI->getTypeLoc(), Sema::AbstractNone);
+  if (TypeSourceInfo *TSI = FD->getTypeSourceInfo())
+    Info.CheckType(FD, TSI->getTypeLoc(), Sema::AbstractNone);
+}
+
+/// Check for invalid uses of an abstract type in a variable0 declaration.
+static void CheckAbstractClassUsage(AbstractUsageInfo &Info,
+                                    VarDecl *VD) {
+  // No need to do the check on definitions, which require that
+  // the type is complete.
+  if (VD->isThisDeclarationADefinition())
+    return;
+
+  Info.CheckType(VD, VD->getTypeSourceInfo()->getTypeLoc(),
+                 Sema::AbstractVariableType);
 }
 
 /// Check for invalid uses of an abstract type within a class definition.
@@ -5923,29 +5936,32 @@ static void CheckAbstractClassUsage(AbstractUsageInfo &Info,
   for (auto *D : RD->decls()) {
     if (D->isImplicit()) continue;
 
-    // Methods and method templates.
-    if (isa<CXXMethodDecl>(D)) {
-      CheckAbstractClassUsage(Info, cast<CXXMethodDecl>(D));
-    } else if (isa<FunctionTemplateDecl>(D)) {
-      FunctionDecl *FD = cast<FunctionTemplateDecl>(D)->getTemplatedDecl();
-      CheckAbstractClassUsage(Info, cast<CXXMethodDecl>(FD));
+    // Step through friends to the befriended declaration.
+    if (auto *FD = dyn_cast<FriendDecl>(D)) {
+      D = FD->getFriendDecl();
+      if (!D) continue;
+    }
+
+    // Functions and function templates.
+    if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+      CheckAbstractClassUsage(Info, FD);
+    } else if (auto *FTD = dyn_cast<FunctionTemplateDecl>(D)) {
+      CheckAbstractClassUsage(Info, FTD->getTemplatedDecl());
 
     // Fields and static variables.
-    } else if (isa<FieldDecl>(D)) {
-      FieldDecl *FD = cast<FieldDecl>(D);
+    } else if (auto *FD = dyn_cast<FieldDecl>(D)) {
       if (TypeSourceInfo *TSI = FD->getTypeSourceInfo())
         Info.CheckType(FD, TSI->getTypeLoc(), Sema::AbstractFieldType);
-    } else if (isa<VarDecl>(D)) {
-      VarDecl *VD = cast<VarDecl>(D);
-      if (TypeSourceInfo *TSI = VD->getTypeSourceInfo())
-        Info.CheckType(VD, TSI->getTypeLoc(), Sema::AbstractVariableType);
+    } else if (auto *VD = dyn_cast<VarDecl>(D)) {
+      CheckAbstractClassUsage(Info, VD);
+    } else if (auto *VTD = dyn_cast<VarTemplateDecl>(D)) {
+      CheckAbstractClassUsage(Info, VTD->getTemplatedDecl());
 
     // Nested classes and class templates.
-    } else if (isa<CXXRecordDecl>(D)) {
-      CheckAbstractClassUsage(Info, cast<CXXRecordDecl>(D));
-    } else if (isa<ClassTemplateDecl>(D)) {
-      CheckAbstractClassUsage(Info,
-                             cast<ClassTemplateDecl>(D)->getTemplatedDecl());
+    } else if (auto *RD = dyn_cast<CXXRecordDecl>(D)) {
+      CheckAbstractClassUsage(Info, RD);
+    } else if (auto *CTD = dyn_cast<ClassTemplateDecl>(D)) {
+      CheckAbstractClassUsage(Info, CTD->getTemplatedDecl());
     }
   }
 }

diff  --git a/clang/test/SemaCXX/abstract.cpp b/clang/test/SemaCXX/abstract.cpp
index c6ee3c6e4b865..e520f2581d1db 100644
--- a/clang/test/SemaCXX/abstract.cpp
+++ b/clang/test/SemaCXX/abstract.cpp
@@ -312,3 +312,42 @@ namespace pr16659 {
     RedundantInit() : A(0) {} // expected-warning {{initializer for virtual base class 'pr16659::A' of abstract class 'RedundantInit' will never be used}}
   };
 }
+
+struct inline_var { // expected-note {{until the closing '}'}}
+  static inline inline_var v = 0; // expected-error {{incomplete type}} expected-warning {{extension}}
+  virtual void f() = 0;
+};
+
+struct var_template {
+  template<typename T>
+  static var_template v; // expected-error {{abstract class}} expected-warning {{extension}}
+  virtual void f() = 0; // expected-note {{unimplemented}}
+};
+
+struct var_template_def { // expected-note {{until the closing '}'}}
+  template<typename T>
+  static inline var_template_def v = {}; // expected-error {{incomplete type}} expected-warning 2{{extension}}
+  virtual void f() = 0;
+};
+
+struct friend_fn {
+  friend void g(friend_fn); // expected-error {{abstract class}}
+  virtual void f() = 0; // expected-note {{unimplemented}}
+};
+
+struct friend_fn_def {
+  friend void g(friend_fn_def) {} // expected-error {{abstract class}}
+  virtual void f() = 0; // expected-note {{unimplemented}}
+};
+
+struct friend_template {
+  template<typename T>
+  friend void g(friend_template); // expected-error {{abstract class}}
+  virtual void f() = 0; // expected-note {{unimplemented}}
+};
+
+struct friend_template_def {
+  template<typename T>
+  friend void g(friend_template_def) {} // expected-error {{abstract class}}
+  virtual void f() = 0; // expected-note {{unimplemented}}
+};

diff  --git a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
index 62b1c166e954c..84b3a542dcaad 100644
--- a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -555,6 +555,20 @@ void insert(Args &&...args);
 void foo() {
   insert(Foo(2, 2, 2)); // expected-error{{no viable constructor or deduction guide}}
 }
+
+namespace PR52139 {
+  struct Abstract {
+    template <class... Ts>
+    struct overloaded : Ts... {
+      using Ts::operator()...;
+    };
+    template <class... Ts>
+    overloaded(Ts...) -> overloaded<Ts...>;
+
+  private:
+    virtual void f() = 0;
+  };
+}
 #else
 
 // expected-no-diagnostics


        


More information about the cfe-commits mailing list