Code review

Fix naming and comment

Author: ganenkokb-yandex

clang/include/clang/AST/ASTImporter.h

@@ -256,14 +256,11 @@ class TypeSourceInfo;
     /// Declaration (from, to) pairs that are known not to be equivalent
     /// (which we have already complained about).
     NonEquivalentDeclSet NonEquivalentDecls;
-    // When template function return type is auto and return type is declared as
-    // typename from template params, there could be cycles in function
-    // importing when function decaration is still the need for return type
-    // declaration import. We have code path for nested types inside function
-    // (see hasReturnTypeDeclaredInside): assuming return type as VoidTy and
-    // calculate it later under UsedDifferentProtoType boolean. This class is
-    // reuse of this approach and make logic lazy - detect cycle - calculate
-    // return type later on.
+    /// A FunctionDecl can have properties that have a reference to the
+    /// function itself and are imported before the function is created. This
+    /// can come for example from auto return type or when template parameters
+    /// are used in the return type or parameters. This member is used to detect
+    /// cyclic import of FunctionDecl objects to avoid infinite recursion.
     std::unique_ptr<FunctionReturnTypeDeclCycleDetector>
         FunctionReturnTypeCycleDetector;
 

clang/lib/AST/ASTImporter.cpp

@@ -1289,34 +1289,34 @@ using namespace clang;
 
 class ASTImporter::FunctionReturnTypeDeclCycleDetector {
 public:
-  class ScopedReturnTypeImport {
+  class DeclCycleMapInserter {
   public:
     // Do not track cycles on D == nullptr.
-    ScopedReturnTypeImport(FunctionReturnTypeDeclCycleDetector &owner,
-                           const FunctionDecl *D)
+    DeclCycleMapInserter(FunctionReturnTypeDeclCycleDetector &owner,
+                         const FunctionDecl *D)
         : CycleDetector(owner), D(D) {
       if (D)
         CycleDetector.FunctionReturnTypeDeclCycles.insert(D);
     }
-    ~ScopedReturnTypeImport() {
+    ~DeclCycleMapInserter() {
       if (D)
         CycleDetector.FunctionReturnTypeDeclCycles.erase(D);
     }
-    ScopedReturnTypeImport(const ScopedReturnTypeImport &) = delete;
-    ScopedReturnTypeImport &operator=(const ScopedReturnTypeImport &) = delete;
+    DeclCycleMapInserter(const DeclCycleMapInserter &) = delete;
+    DeclCycleMapInserter &operator=(const DeclCycleMapInserter &) = delete;
 
   private:
     FunctionReturnTypeDeclCycleDetector &CycleDetector;
     const FunctionDecl *D;
   };
 
-  ScopedReturnTypeImport DetectImportCycles(const FunctionDecl *D) {
-    if (!IsCycle(D))
-      return ScopedReturnTypeImport(*this, D);
-    return ScopedReturnTypeImport(*this, nullptr);
+  DeclCycleMapInserter detectImportCycle(const FunctionDecl *D) {
+    if (!isCycle(D))
+      return DeclCycleMapInserter(*this, D);
+    return DeclCycleMapInserter(*this, nullptr);
   }
 
-  bool IsCycle(const FunctionDecl *D) const {
+  bool isCycle(const FunctionDecl *D) const {
     return FunctionReturnTypeDeclCycles.find(D) !=
            FunctionReturnTypeDeclCycles.end();
   }
@@ -4076,7 +4076,7 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
     // Reuse this approach for auto return types declared as typenames from
     // template pamams, tracked in FunctionReturnTypeCycleDetector.
     if (hasReturnTypeDeclaredInside(D) ||
-        Importer.FunctionReturnTypeCycleDetector->IsCycle(D)) {
+        Importer.FunctionReturnTypeCycleDetector->isCycle(D)) {
       FromReturnTy = Importer.getFromContext().VoidTy;
       UsedDifferentProtoType = true;
     }
@@ -4100,7 +4100,7 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   Error Err = Error::success();
   auto ScopedReturnTypeDeclCycleDetector =
-      Importer.FunctionReturnTypeCycleDetector->DetectImportCycles(D);
+      Importer.FunctionReturnTypeCycleDetector->detectImportCycle(D);
   auto T = importChecked(Err, FromTy);
   auto TInfo = importChecked(Err, FromTSI);
   auto ToInnerLocStart = importChecked(Err, D->getInnerLocStart());

clang/unittests/AST/ASTImporterTest.cpp

@@ -3204,7 +3204,7 @@ TEST_P(ImportExpr, UnresolvedMemberExpr) {
                  compoundStmt(has(callExpr(has(unresolvedMemberExpr())))))))));
 }
 
-TEST_P(ImportExpr, CycleInAutoTemplateSpec) {
+TEST_P(ImportDecl, CycleInAutoTemplateSpec) {
   MatchVerifier<Decl> Verifier;
   const char *Code = R"(
   template <class _CharT>