[clang] [clang-tools-extra] [Sema]Report error for non-base types in constructor initializers before causing issue in initializer order (PR #201379)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Gábor Spaits (spaits)

<details>
<summary>Changes</summary>

Fixes #<!-- -->201593 .

---
Full diff: https://github.com/llvm/llvm-project/pull/201379.diff


4 Files Affected:

- (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp (+1-1) 
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+27-24) 
- (modified) clang/test/SemaCXX/constructor-initializer.cpp (+18) 
- (modified) clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp (-10) 


``````````diff
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
index 55963eae2c1b5..e1724c3c1e338 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
@@ -647,7 +647,7 @@ namespace gh192510 {
   };
 
   template<typename T>
-  class X: public Base {
+  class X: public Base, private C<T> {
     using INT = C<T>;
 
     X(INT i) : INT(i) {} // no crash
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9f9e76bf336b0..6f5baf9166287 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/ComparisonCategories.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DynamicRecursiveASTVisitor.h"
@@ -4819,33 +4820,35 @@ Sema::BuildBaseInitializer(QualType BaseType, TypeSourceInfo *BaseTInfo,
       return true;
   }
 
+  if (!Dependent &&
+      declaresSameEntity(ClassDecl, BaseType->getAsCXXRecordDecl()))
+    return BuildDelegatingInitializer(BaseTInfo, Init, ClassDecl);
+
   // Check for direct and virtual base classes.
   const CXXBaseSpecifier *DirectBaseSpec = nullptr;
   const CXXBaseSpecifier *VirtualBaseSpec = nullptr;
-  if (!Dependent) {
-    if (declaresSameEntity(ClassDecl, BaseType->getAsCXXRecordDecl()))
-      return BuildDelegatingInitializer(BaseTInfo, Init, ClassDecl);
-
-    FindBaseInitializer(*this, ClassDecl, BaseType, DirectBaseSpec,
-                        VirtualBaseSpec);
-
-    // C++ [base.class.init]p2:
-    // Unless the mem-initializer-id names a nonstatic data member of the
-    // constructor's class or a direct or virtual base of that class, the
-    // mem-initializer is ill-formed.
-    if (!DirectBaseSpec && !VirtualBaseSpec) {
-      // If the class has any dependent bases, then it's possible that
-      // one of those types will resolve to the same type as
-      // BaseType. Therefore, just treat this as a dependent base
-      // class initialization.  FIXME: Should we try to check the
-      // initialization anyway? It seems odd.
-      if (ClassDecl->hasAnyDependentBases())
-        Dependent = true;
-      else
-        return Diag(BaseLoc, diag::err_not_direct_base_or_virtual)
-               << BaseType << Context.getCanonicalTagType(ClassDecl)
-               << BaseTInfo->getTypeLoc().getSourceRange();
-    }
+
+  FindBaseInitializer(*this, ClassDecl, BaseType, DirectBaseSpec,
+                      VirtualBaseSpec);
+
+  // C++ [base.class.init]p2:
+  // Unless the mem-initializer-id names a nonstatic data member of the
+  // constructor's class or a direct or virtual base of that class, the
+  // mem-initializer is ill-formed.
+  if (!DirectBaseSpec && !VirtualBaseSpec) {
+    // If the class has any dependent bases, then it's possible that one of
+    // those types will resolve to the same type as BaseType. Therefore, just
+    // treat this as a dependent base class initialization.
+    // FIXME: Should we try to check the initialization anyway? It seems odd.
+    if (ClassDecl->hasAnyDependentBases())
+      Dependent = true;
+    // We may have a delegating initializer here but in a dependent context.
+    // Since that is also a type, that isn't a direct or virtual base of the
+    // instantiated type. That will be handled later.
+    else if (!declaresSameEntity(ClassDecl, BaseType->getAsCXXRecordDecl()))
+      return Diag(BaseLoc, diag::err_not_direct_base_or_virtual)
+             << BaseType << Context.getCanonicalTagType(ClassDecl)
+             << BaseTInfo->getTypeLoc().getSourceRange();
   }
 
   if (Dependent) {
diff --git a/clang/test/SemaCXX/constructor-initializer.cpp b/clang/test/SemaCXX/constructor-initializer.cpp
index 96be8dda97735..ff6374d6eb6ef 100644
--- a/clang/test/SemaCXX/constructor-initializer.cpp
+++ b/clang/test/SemaCXX/constructor-initializer.cpp
@@ -323,3 +323,21 @@ A f2(const B &b) {
   return b; // expected-error {{no matching constructor for initialization of 'B'}}
 }
 }
+
+namespace PR7179 {
+struct X
+{
+  struct Y
+  {
+    template <class T> Y(T x) : X(x) { } // expected-error {{type 'X' is not a direct or virtual base of 'PR7179::X::Y'}}
+  };
+};
+}
+
+namespace PR201379 {
+struct S1 {
+  template<typename T>
+  S1(T) : d(), T()  {} // expected-error {{type 'T' is not a direct or virtual base of 'PR201379::S1'}}
+  int d;
+};
+}
diff --git a/clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp b/clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
index 0613338945978..b0153c6984b5b 100644
--- a/clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
+++ b/clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
@@ -120,16 +120,6 @@ namespace test3 {
   };
 }
 
-namespace PR7179 {
-  struct X
-  {
-    struct Y
-    {
-      template <class T> Y(T x) : X(x) { }
-    };
-  };
-}
-
 namespace test3 {
   struct foo {
     struct {

``````````

</details>


https://github.com/llvm/llvm-project/pull/201379