balazske updated this revision to Diff 449668.
balazske added a comment.

- Added a new test
- Call the visitor only if the function has auto return type


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130705/new/

https://reviews.llvm.org/D130705

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -6318,64 +6318,195 @@
   EXPECT_EQ(ToEPI.ExceptionSpec.SourceDecl, ToCtor);
 }
 
-struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {};
+struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {
+  void testImport(llvm::StringRef Code, clang::TestLanguage Lang = Lang_CXX14) {
+    Decl *FromTU = getTuDecl(Code, Lang, "input0.cc");
+    FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
+        FromTU, functionDecl(hasName("foo")));
+
+    FunctionDecl *To = Import(From, Lang);
+    EXPECT_TRUE(To);
+    // We check here only that the type is auto type.
+    // These tests are to verify that no crash happens.
+    // The crash possibility is the presence of a reference to a declaration
+    // in the function's body from the return type, if the function has auto
+    // return type.
+    EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+  }
+};
+
+TEST_P(ImportAutoFunctions, ReturnWithAutoUnresolvedArg) {
+  testImport(
+      R"(
+      template<int A>
+      auto foo() {
+        return 22;
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithTemplateTemplateArg) {
+  // FIXME: Is it possible to have the template arg inside the function?
+  testImport(
+      R"(
+      template<int> struct Tmpl {};
+      template<template<int> class> struct TmplTmpl {};
+      auto foo() {
+        return TmplTmpl<Tmpl>();
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithDeclarationTemplateArg) {
+  // FIXME: Is it possible to have the template arg inside the function?
+  testImport(
+      R"(
+      template<const int *> struct Tmpl {};
+      int A[10];
+      auto foo() {
+        return Tmpl<A>();
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithNullPtrTemplateArg) {
+  testImport(
+      R"(
+      template<int *> struct Tmpl {};
+      auto foo() {
+        constexpr int* A = nullptr;
+        return Tmpl<A>();
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithIntegralTemplateArg) {
+  testImport(
+      R"(
+      template<int> struct Tmpl {};
+      auto foo() {
+        using Int = int;
+        constexpr Int A = 7;
+        return Tmpl<A>();
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithDecltypeTypeDeclaredInside) {
+  testImport(
+      R"(
+      template<class> struct Tmpl {};
+      auto foo() {
+        struct X {};
+        X x;
+        return Tmpl<decltype(x)>();
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithUsingTypeDeclaredInside) {
+  testImport(
+      R"(
+      template<class> struct Tmpl {};
+      namespace A { struct X {}; }
+      auto foo() {
+        using A::X;
+        return Tmpl<X>();
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithArrayTypeDeclaredInside) {
+  testImport(
+      R"(
+      template<class> struct Tmpl {};
+      auto foo() {
+        struct X {};
+        return Tmpl<X[10]>();
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithArraySizeExprDeclaredInside) {
+  testImport(
+      R"(
+      template<class> struct Tmpl {};
+      auto foo() {
+        constexpr int S = 10;
+        return Tmpl<int[S]>();
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithPackArgDeclaredInside) {
+  testImport(
+      R"(
+      template<class ...> struct Tmpl {};
+      auto foo() {
+        using X = bool;
+        return Tmpl<int, X>();
+      }
+      )");
+}
 
 TEST_P(ImportAutoFunctions, ReturnWithTemplateWithIntegerArgDeclaredInside) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
       template<int> struct Tmpl {};
       auto foo() {
         constexpr int X = 1;
         return Tmpl<X>();
       }
-      )",
-      Lang_CXX14, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
-      FromTU, functionDecl(hasName("foo")));
+      )");
+}
 
-  FunctionDecl *To = Import(From, Lang_CXX14);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithPtrToStructDeclaredInside) {
+  testImport(
+      R"(
+      template<class> struct Tmpl {};
+      auto foo() {
+        struct X {};
+        return Tmpl<X *>();
+      }
+      )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithRefToStructDeclaredInside) {
+  testImport(
+      R"(
+      template<class> struct Tmpl {};
+      struct X {};
+      auto foo() {
+        using Y = X;
+        return Tmpl<Y &>();
+      }
+      )");
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithTemplateWithStructDeclaredInside1) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
       template<class> struct Tmpl {};
       auto foo() {
         struct X {};
         return Tmpl<X>();
       }
-      )",
-      Lang_CXX14, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
-      FromTU, functionDecl(hasName("foo")));
-
-  FunctionDecl *To = Import(From, Lang_CXX14);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+      )");
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithTemplateWithStructDeclaredInside2) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
       template<class> struct Tmpl {};
       auto foo() {
         struct X {};
         return Tmpl<Tmpl<X>>();
       }
-      )",
-      Lang_CXX14, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
-      FromTU, functionDecl(hasName("foo")));
-
-  FunctionDecl *To = Import(From, Lang_CXX14);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+      )");
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithTemplateWithTypedefDeclaredInside) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
       template<class> struct Tmpl {};
       auto foo() {
@@ -6383,14 +6514,7 @@
         using x_type = X;
         return Tmpl<x_type>();
       }
-      )",
-      Lang_CXX14, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
-      FromTU, functionDecl(hasName("foo")));
-
-  FunctionDecl *To = Import(From, Lang_CXX14);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+      )");
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithTypedefDeclaredInside) {
@@ -6429,20 +6553,13 @@
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithStructDeclaredInside) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
       auto foo() {
         struct X {};
         return X();
       }
-      )",
-      Lang_CXX14, "input0.cc");
-  FunctionDecl *From =
-      FirstDeclMatcher<FunctionDecl>().match(FromTU, functionDecl());
-
-  FunctionDecl *To = Import(From, Lang_CXX14);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+      )");
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithStructDeclaredInside2) {
@@ -6463,45 +6580,53 @@
   EXPECT_TRUE(isa<AutoType>(FPT->getReturnType()));
 }
 
-TEST_P(ImportAutoFunctions, ReturnWithTypedefToStructDeclaredInside) {
+TEST_P(ImportAutoFunctions, ReturnWithStructDeclaredInside3) {
   Decl *FromTU = getTuDecl(
       R"(
-      auto foo() {
+      struct S {
+        constexpr auto foo();
+      };
+      constexpr auto S::foo() {
         struct X {};
-        using Y = X;
-        return Y();
+        return X();
       }
       )",
       Lang_CXX14, "input0.cc");
-  FunctionDecl *From =
-      FirstDeclMatcher<FunctionDecl>().match(FromTU, functionDecl());
+  FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("foo"), unless(hasBody(stmt()))));
+  ASSERT_FALSE(From->isThisDeclarationADefinition());
 
-  FunctionDecl *To = Import(From, Lang_CXX14);
+  FunctionDecl *To = Import(From, Lang_CXX17);
   EXPECT_TRUE(To);
   EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+  EXPECT_FALSE(To->isThisDeclarationADefinition());
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTypedefToStructDeclaredInside) {
+  testImport(
+      R"(
+      auto foo() {
+        struct X {};
+        using Y = X;
+        return Y();
+      }
+      )");
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithStructDeclaredNestedInside) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
       auto foo() {
         struct X { struct Y{}; };
         return X::Y();
       }
-      )",
-      Lang_CXX14, "input0.cc");
-  FunctionDecl *From =
-      FirstDeclMatcher<FunctionDecl>().match(FromTU, functionDecl());
-
-  FunctionDecl *To = Import(From, Lang_CXX14);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+      )");
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithInternalLambdaType) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
-      auto f() {
+      auto foo() {
         auto l = []() {
           struct X {};
           return X();
@@ -6509,68 +6634,44 @@
         return l();
       }
       )",
-      Lang_CXX17, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
-      FromTU, functionDecl(hasName("f")));
-
-  FunctionDecl *To = Import(From, Lang_CXX17);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+      Lang_CXX17);
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithTypeInIf) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
-      auto f() {
+      auto foo() {
         if (struct X {} x; true)
           return X();
         else
           return X();
       }
       )",
-      Lang_CXX17, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
-      FromTU, functionDecl(hasName("f")));
-
-  FunctionDecl *To = Import(From, Lang_CXX17);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+      Lang_CXX17);
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithTypeInFor) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
-      auto f() {
+      auto foo() {
         for (struct X {} x;;)
           return X();
       }
       )",
-      Lang_CXX17, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
-      FromTU, functionDecl(hasName("f")));
-
-  FunctionDecl *To = Import(From, Lang_CXX17);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+      Lang_CXX17);
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithTypeInSwitch) {
-  Decl *FromTU = getTuDecl(
+  testImport(
       R"(
-      auto f() {
+      auto foo() {
         switch (struct X {} x; 10) {
         case 10:
           return X();
         }
       }
       )",
-      Lang_CXX17, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
-      FromTU, functionDecl(hasName("f")));
-
-  FunctionDecl *To = Import(From, Lang_CXX17);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
+      Lang_CXX17);
 }
 
 struct ImportSourceLocations : ASTImporterOptionSpecificTestBase {};
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3248,56 +3248,162 @@
   return false;
 }
 
-static bool hasTypeDeclaredInsideFunction(QualType T, const FunctionDecl *FD) {
-  if (T.isNull())
+namespace {
+/// Check if a type has any reference to a declaration that is inside the body
+/// of a function.
+/// The \c CheckType(QualType) function should be used to determine
+/// this property.
+///
+/// The type visitor visits one type object only (not recursive).
+/// To find all referenced declarations we must discover all type objects until
+/// the canonical type is reached (walk over typedef and similar objects). This
+/// is done by loop over all "sugar" type objects. For every such type we must
+/// check all declarations that are referenced from it. For this check the
+/// visitor is used. In the visit functions all referenced declarations except
+/// the one that follows in the sugar chain (if any) must be checked. For this
+/// check the same visitor is re-used (it has no state-dependent data).
+///
+/// The visit functions have 3 possible return values:
+///  - True, found a declaration inside \c ParentDC.
+///  - False, found declarations only outside \c ParentDC and it is not possible
+///    to find more declarations (the "sugar" chain does not continue).
+///  - Empty optional value, found no declarations or only outside \c ParentDC,
+///    but it is possible to find more declarations in the type "sugar" chain.
+/// The loop over the "sugar" types can be implemented by using type visit
+/// functions only (call \c CheckType with the desugared type). With the current
+/// solution no visit function is needed if the type has only a desugared type
+/// as data.
+class IsTypeDeclaredInsideVisitor
+    : public TypeVisitor<IsTypeDeclaredInsideVisitor, Optional<bool>> {
+public:
+  IsTypeDeclaredInsideVisitor(const FunctionDecl *ParentDC)
+      : ParentDC(ParentDC) {}
+
+  bool CheckType(QualType T) {
+    // Check the chain of "sugar" types.
+    // The "sugar" types are typedef or similar types that have the same
+    // canonical type.
+    if (Optional<bool> Res = Visit(T.getTypePtr()))
+      return *Res;
+    QualType DsT =
+        T.getSingleStepDesugaredType(ParentDC->getParentASTContext());
+    while (DsT != T) {
+      if (Optional<bool> Res = Visit(DsT.getTypePtr()))
+        return *Res;
+      T = DsT;
+      DsT = T.getSingleStepDesugaredType(ParentDC->getParentASTContext());
+    }
     return false;
+  }
+
+  Optional<bool> VisitTypedefType(const TypedefType *T) {
+    const TypedefNameDecl *TD = T->getDecl();
+    assert(TD);
+    return isAncestorDeclContextOf(ParentDC, TD);
+  }
+
+  Optional<bool>
+  VisitTemplateSpecializationType(const TemplateSpecializationType *T) {
+    for (const auto &Arg : T->template_arguments())
+      if (checkTemplateArgument(Arg))
+        return true;
+    // This type is a "sugar" to a record type, it can have a desugared type.
+    return {};
+  }
+
+  Optional<bool> VisitTagType(const TagType *T) {
+    return isAncestorDeclContextOf(ParentDC, T->getDecl());
+  }
+
+  Optional<bool> VisitPointerType(const PointerType *T) {
+    return CheckType(T->getPointeeType());
+  }
+
+  Optional<bool> VisitReferenceType(const ReferenceType *T) {
+    return CheckType(T->getPointeeTypeAsWritten());
+  }
+
+  Optional<bool> VisitConstantArrayType(const ConstantArrayType *T) {
+    if (T->getSizeExpr() && isAncestorDeclContextOf(ParentDC, T->getSizeExpr()))
+      return true;
+
+    return CheckType(T->getElementType());
+  }
+
+  Optional<bool> VisitUsingType(const UsingType *T) {
+    if (T->getFoundDecl() &&
+        isAncestorDeclContextOf(ParentDC, T->getFoundDecl()))
+      return true;
 
-  auto CheckTemplateArgument = [FD](const TemplateArgument &Arg) {
+    return {};
+  }
+
+  Optional<bool> VisitVariableArrayType(const VariableArrayType *T) {
+    llvm_unreachable(
+        "Variable array should not occur in deduced return type of a function");
+  }
+
+  Optional<bool> VisitIncompleteArrayType(const IncompleteArrayType *T) {
+    llvm_unreachable("Incomplete array should not occur in deduced return type "
+                     "of a function");
+  }
+
+  Optional<bool> VisitDependentArrayType(const IncompleteArrayType *T) {
+    llvm_unreachable("Dependent array should not occur in deduced return type "
+                     "of a function");
+  }
+
+private:
+  const DeclContext *const ParentDC;
+
+  bool checkTemplateArgument(const TemplateArgument &Arg) {
     switch (Arg.getKind()) {
+    case TemplateArgument::Null:
+      return false;
+    case TemplateArgument::Integral:
+      return CheckType(Arg.getIntegralType());
     case TemplateArgument::Type:
-      return hasTypeDeclaredInsideFunction(Arg.getAsType(), FD);
+      return CheckType(Arg.getAsType());
     case TemplateArgument::Expression:
-      return isAncestorDeclContextOf(FD, Arg.getAsExpr());
-    default:
-      // FIXME: Handle other argument kinds.
+      return isAncestorDeclContextOf(ParentDC, Arg.getAsExpr());
+    case TemplateArgument::Declaration:
+      // FIXME: The declaration in this case is not allowed to be in a function?
+      return isAncestorDeclContextOf(ParentDC, Arg.getAsDecl());
+    case TemplateArgument::NullPtr:
+      // FIXME: The type is not allowed to be in the function?
+      return CheckType(Arg.getNullPtrType());
+    case TemplateArgument::Pack:
+      llvm_unreachable(
+          "Add the code below and test for it if this place is reached");
+      // for (const auto &PackArg: Arg.getPackAsArray())
+      //   if (checkTemplateArgument(PackArg))
+      //     return true;
+      // return false;
+    case TemplateArgument::Template:
+      // Templates can not be defined locally in functions.
+      // A template passed as argument can be not in ParentDC.
+      return false;
+    case TemplateArgument::TemplateExpansion:
+      // Templates can not be defined locally in functions.
+      // A template passed as argument can be not in ParentDC.
       return false;
     }
   };
-
-  if (const auto *RecordT = T->getAs<RecordType>()) {
-    const RecordDecl *RD = RecordT->getDecl();
-    assert(RD);
-    if (isAncestorDeclContextOf(FD, RD)) {
-      assert(RD->getLexicalDeclContext() == RD->getDeclContext());
-      return true;
-    }
-    if (const auto *RDTempl = dyn_cast<ClassTemplateSpecializationDecl>(RD))
-      if (llvm::count_if(RDTempl->getTemplateArgs().asArray(),
-                         CheckTemplateArgument))
-        return true;
-    // Note: It is possible that T can be get as both a RecordType and a
-    // TemplateSpecializationType.
-  }
-  if (const auto *TST = T->getAs<TemplateSpecializationType>())
-    return llvm::count_if(TST->template_arguments(), CheckTemplateArgument);
-
-  return false;
-}
+};
+} // namespace
 
 bool ASTNodeImporter::hasAutoReturnTypeDeclaredInside(FunctionDecl *D) {
   QualType FromTy = D->getType();
   const auto *FromFPT = FromTy->getAs<FunctionProtoType>();
   assert(FromFPT && "Must be called on FunctionProtoType");
-  if (const AutoType *AutoT = FromFPT->getReturnType()->getContainedAutoType())
-    return hasTypeDeclaredInsideFunction(AutoT->getDeducedType(), D);
-  if (const auto *TypedefT = FromFPT->getReturnType()->getAs<TypedefType>()) {
-    const TypedefNameDecl *TD = TypedefT->getDecl();
-    assert(TD);
-    if (isAncestorDeclContextOf(D, TD)) {
-      assert(TD->getLexicalDeclContext() == TD->getDeclContext());
-      return true;
-    }
+
+  QualType RetT = FromFPT->getReturnType();
+  if (isa<AutoType>(RetT.getTypePtr())) {
+    FunctionDecl *Def = D->getDefinition();
+    IsTypeDeclaredInsideVisitor Visitor(Def ? Def : D);
+    return Visitor.CheckType(RetT);
   }
+
   return false;
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to