Hi John,

Thanks for taking the time to review my changes. The following patch should 
address all of your concerns. I have clarified that I want to traverse all of 
the template instantiations (not just the explicit ones).

  /// This visitor always recurses in the declarator of explicit
  /// template instantiations, such as for example "template class
  /// set<int>'. But by default, it will not recurse in the
  /// instantiations (implicit or explicit) of the template (function or
  /// class) themselves since the instatiated AST isn't written in the
  /// source code anywhere. This behavior can be changed by overriding
  /// shouldVisitTemplateInstantiations() in the derived class to return
  /// true.

Unfortunately, while testing my changes, I have come across an issue and I 
don’t know what the correct solution is. The following code snippet (inspired 
from the Intel TBB header files) demonstrates the issue:


—————————————————
// This is fine.
class A 
{
    friend class A;
}

// This class template definiton is also fine...
template<typename Container>
class vector_iterator
{
    template <typename C> friend class vector_iterator;
};

// ... until it gets instantiated.
vector_iterator<int> it_int;
--------------------------------

It will throw the RecursiveASTVisitor in an infinite loop when 
shouldVisitTemplateInstantiations() == true. The attached file tracing.txt 
contains a trace of the visitor in action and points out to cause of the 
problem:

getFriendDecl() on the FriendDecl contained in the ClassTemplateSpecialization 
for vector_iterator<int> points back to the exact same 
ClassTemplateSpecialization object. Thus, the RecuriveASTVisitor recurses 
forever.

1) Is this behavior expected or is this a bug ?
2) If the behavior is expected, what’s the best way to solve this ?
   a) Not traversing FriendDecl contained in ClassTemplateSpecializations ?
   b) Keeping track of AST nodes that have already been visited (potentially 
costly) ?


Cheers,
Benoit


Le 2010-08-26 à 16:48, John McCall a écrit :

> On Aug 26, 2010, at 12:24 PM, Benoit Belley wrote:
>> I have modified the RecursiveASTVisitor so that it can now optionally visit 
>> every template specialization (class and fucntion templates, implicit and 
>> explicit instantiations). 
>> 
>> By default this visitor will not recurse in template specializations since 
>> the instatiated class isn’t written in the source code anywhere. This 
>> behavior can now be changed by calling 
>> setVisitingTemplateSpecializations(true) before starting the traversal.
> 
> The idiomatic way of doing this with the CRTP is to call methods on the 
> derived class instead of consulting flags.
> 
> That is, TRAVERSE_DECL(ClassTemplateDecl) should 
> TRY_TO(TraverseImplicitInstantantiations(D)), TraverseImplicitInstantiations 
> should check getDerived().shouldTraverseImplicitInstantiations() before 
> proceeding, and shouldTraverseImplicitInstantiations() should return false 
> (which you would override in your subclass, of course).
> 
> The way to check whether a function template specialization is an implicit 
> instantiation is to consult 
> getTemplateSpecializationInfo()->getTemplateSpecializationKind().  For a 
> class template specialization, it's just getSpecializationKind().
> 
> John.



Index: include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- include/clang/AST/RecursiveASTVisitor.h     (revision 111934)
+++ include/clang/AST/RecursiveASTVisitor.h     (working copy)
@@ -123,12 +123,26 @@
 /// users may override Traverse* and WalkUpFrom* to implement custom
 /// traversal strategies.  Returning false from one of these overridden
 /// functions will abort the entire traversal.
+///
+/// This visitor always recurses in the declarator of explicit
+/// template instantiations, such as for example "template class
+/// set<int>'. But by default, it will not recurse in the
+/// instantiations (implicit or explicit) of the template (function or
+/// class) themselves since the instatiated AST isn't written in the
+/// source code anywhere. This behavior can be changed by overriding
+/// shouldVisitTemplateInstantiations() in the derived class to return
+/// true.
+
 template<typename Derived>
 class RecursiveASTVisitor {
 public:
   /// \brief Return a reference to the derived class.
   Derived &getDerived() { return *static_cast<Derived*>(this); }
 
+  /// \brief Return whether this visitor should recurse into
+  /// template instantiations.
+  bool shouldVisitTemplateInstantiations() const { return false; }
+  
   /// \brief Recursively visit a statement or expression, by
   /// dispatching to Traverse*() based on the argument's dynamic type.
   ///
@@ -351,7 +365,9 @@
 private:
   // These are helper methods used by more than one Traverse* method.
   bool TraverseTemplateParameterListHelper(TemplateParameterList *TPL);
-  bool TraverseTemplateArgumentLocsHelper(const TemplateArgumentLoc *TAL,
+  bool TraverseImplicitClassInstantiations(ClassTemplateDecl* D);
+  bool TraverseFunctionInstantiations(FunctionTemplateDecl* D) ;
+  bool TraverseTemplateArgumentLocsHelper(const TemplateArgumentLoc *TAL, 
                                           unsigned Count);
   bool TraverseArrayTypeLocHelper(ArrayTypeLoc TL);
   bool TraverseRecordHelper(RecordDecl *D);
@@ -1092,19 +1108,96 @@
   return true;
 }
 
+// A helper method for traversing the implicit instantiations of a
+// class.
+template<typename Derived>
+bool RecursiveASTVisitor<Derived>::TraverseImplicitClassInstantiations(
+  ClassTemplateDecl* D) {
+  // By default, we do not traverse the instantiations of
+  // class templates since they do not apprear in the user code. The
+  // following code optionally traverses them.
+  if (!getDerived().shouldVisitTemplateInstantiations())
+    return true;
+  
+  ClassTemplateDecl::spec_iterator end = D->spec_end();
+  for (ClassTemplateDecl::spec_iterator it = D->spec_begin(); it != end; ++it) 
{
+    ClassTemplateSpecializationDecl* SD = *it;
+
+    switch (SD->getSpecializationKind()) {
+    case TSK_Undeclared:
+      assert(false && "Semantically correct ASTs cannot contain "
+             "undeclared specializations.");
+    case TSK_ImplicitInstantiation: 
+      TRY_TO(TraverseClassTemplateSpecializationDecl(*it));
+      break;
+    case TSK_ExplicitInstantiationDeclaration:
+    case TSK_ExplicitInstantiationDefinition:
+    case TSK_ExplicitSpecialization:
+      break;
+    default:
+      assert(false && "Unknown specialization kind.");
+    }
+  }
+
+  return true;
+}
+  
 DEF_TRAVERSE_DECL(ClassTemplateDecl, {
     TRY_TO(TraverseDecl(D->getTemplatedDecl()));
     TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));
-    // We should not traverse the specializations/partial
-    // specializations.  Those will show up in other contexts.
-    // getInstantiatedFromMemberTemplate() is just a link from a
-    // template instantiation back to the template from which it was
-    // instantiated, and thus should not be traversed either.
+    // Explicit class intantiations and specializations will be
+    // traversed from the context of their declaration. There
+    // is therefore no need to traverse them for here.
+    TRY_TO(TraverseImplicitClassInstantiations(D));
+
+    // Partial specializations are already traversed from their
+    // definition context. There is therefore no need to traverse them
+    // from here.
+    
+    // Note that getInstantiatedFromMemberTemplate() is just a link
+    // from a template instantiation back to the template from which
+    // it was instantiated, and thus should not be traversed.
   })
 
+// A helper method for traversing the instantiations of a
+// function while skipping its specializations.
+template<typename Derived>
+bool RecursiveASTVisitor<Derived>::TraverseFunctionInstantiations(
+  FunctionTemplateDecl* D) {
+  // By default, we do not traverse the instantiations of
+  // function templates since they do not apprear in the user code. The
+  // following code optionally traverses them.
+  if (!getDerived().shouldVisitTemplateInstantiations())
+    return true;
+  
+  FunctionTemplateDecl::spec_iterator end = D->spec_end();
+  for (FunctionTemplateDecl::spec_iterator it = D->spec_begin(); it != end; 
++it) {
+    FunctionDecl* FD = *it;
+    switch (FD->getTemplateSpecializationKind()) {
+    case TSK_Undeclared:           // Declaration of the template definition.
+    case TSK_ImplicitInstantiation: 
+    case TSK_ExplicitInstantiationDeclaration:
+    case TSK_ExplicitInstantiationDefinition:
+      TRY_TO(TraverseFunctionDecl(*it));
+      break;
+
+    case TSK_ExplicitSpecialization:
+      break;
+    default:
+      assert(false && "Unknown specialization kind.");
+    }
+  }
+  
+  return true;
+}
+  
 DEF_TRAVERSE_DECL(FunctionTemplateDecl, {
     TRY_TO(TraverseDecl(D->getTemplatedDecl()));
     TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));
+    // Explicit function specializations will be traversed from the
+    // context of their declaration. There is therefore no need to
+    // traverse them for here.
+    TRY_TO(TraverseFunctionInstantiations(D));
   })
 
 DEF_TRAVERSE_DECL(TemplateTemplateParmDecl, {
@@ -1192,18 +1285,22 @@
   })
 
 DEF_TRAVERSE_DECL(ClassTemplateSpecializationDecl, {
-    // For implicit instantiations ("set<int> x;"), we don't want to
-    // recurse at all, since the instatiated class isn't written in
-    // the source code anywhere.  (Note the instatiated *type* --
-    // set<int> -- is written, and will still get a callback of
-    // TemplateSpecializationType).  For explicit instantiations
-    // ("template set<int>;"), we do need a callback, since this
-    // is the only callback that's made for this instantiation.
-    // We use getTypeAsWritten() to distinguish.
-    // FIXME: see how we want to handle template specializations.
-    if (TypeSourceInfo *TSI = D->getTypeAsWritten())
-      TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
-    return true;
+    if (shouldVisitTemplateInstantiations()) {
+      TRY_TO(TraverseCXXRecordHelper(D));
+    }
+    else {
+      // For implicit instantiations ("set<int> x;"), we don't want to
+      // recurse at all, since the instatiated class isn't written in
+      // the source code anywhere.  (Note the instatiated *type* --
+      // set<int> -- is written, and will still get a callback of
+      // TemplateSpecializationType).  For explicit instantiations
+      // ("template set<int>;"), we do need a callback, since this
+      // is the only callback that's made for this instantiation.
+      // We use getTypeAsWritten() to distinguish.
+      // FIXME: see how we want to handle template specializations.
+      if (TypeSourceInfo *TSI = D->getTypeAsWritten())
+        TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
+    }
   })
 
 template <typename Derived>



Attachment: RecursiveASTVisitor-TemplSpec.patch
Description: RecursiveASTVisitor-TemplSpec.patch




Visiting Decl TranslationUnit @0x10501ecd0 () { 
  Visiting Decl Typedef @0x10501f070 (__int128_t) { 
  }
  Visiting Decl Typedef @0x10501f0c0 (__uint128_t) { 
  }
  Visiting Decl CXXRecord @0x10501f100 (__va_list_tag) { 
    Visiting Decl CXXRecord @0x10501f260 (__va_list_tag::__va_list_tag) { 
      "***Implicit declaration ignored***"
    }
    Visiting Decl Field @0x10501f300 (__va_list_tag::gp_offset) { 
    }
    Visiting Decl Field @0x10501f360 (__va_list_tag::fp_offset) { 
    }
    Visiting Decl Field @0x10501f3c0 (__va_list_tag::overflow_arg_area) { 
    }
    Visiting Decl Field @0x10501f420 (__va_list_tag::reg_save_area) { 
    }
  }
  Visiting Decl Typedef @0x10501f4c0 (__va_list_tag) { 
  }
  Visiting Decl Typedef @0x10501f6b0 (__builtin_va_list) { 
  }
  Visiting Decl CXXRecord @0x10501f6f0 (A) { 
    Visiting Decl CXXRecord @0x10501f850 (A::A) { 
      "***Implicit declaration ignored***"
    }
    Visiting Decl Friend @0x10501f930 () { 
    }
  }
  Visiting Decl ClassTemplate @0x105046900 (vector_iterator) { 
    Visiting Decl CXXRecord @0x105046870 (vector_iterator) { 
      Visiting Decl CXXRecord @0x105046b10 (vector_iterator::vector_iterator) { 
        "***Implicit declaration ignored***"
      }
      Visiting Decl Friend @0x105046e70 () { 
        Visiting Decl ClassTemplate @0x105046cf0 (vector_iterator) { 
          Visiting Decl CXXRecord @0x105046c60 (vector_iterator) { 
          }
          Visiting Decl TemplateTypeParm @0x105046c00 (C) { 
          }
        }
      }
    }
    Visiting Decl TemplateTypeParm @0x105046810 (Container) { 
    }
    Visiting Decl ClassTemplateSpecialization @0x105046ec0 (vector_iterator) { 
      getSpecializationKind() = 1
      Visiting Decl CXXRecord @0x105047190 
(vector_iterator<int>::vector_iterator) { 
        "***Implicit declaration ignored***"
      }
      Visiting Decl Friend @0x105047350 () { 
        Visiting Decl ClassTemplate @0x105047310 (vector_iterator) { 
          Visiting Decl CXXRecord @0x105047280 (vector_iterator) { 
          }
          Visiting Decl TemplateTypeParm @0x105047220 
(vector_iterator<int>::<anonymous>) { 
          }
          Visiting Decl ClassTemplateSpecialization @0x105046ec0 
(vector_iterator) { 
            getSpecializationKind() = 1
            Visiting Decl CXXRecord @0x105047190 
(vector_iterator<int>::vector_iterator) { 
              "***Implicit declaration ignored***"
            }
            Visiting Decl Friend @0x105047350 () { 
              Visiting Decl ClassTemplate @0x105047310 (vector_iterator) { 
                Visiting Decl CXXRecord @0x105047280 (vector_iterator) { 
                }
                Visiting Decl TemplateTypeParm @0x105047220 
(vector_iterator<int>::<anonymous>) { 
                }
                Visiting Decl ClassTemplateSpecialization @0x105046ec0 
(vector_iterator) { 
                  getSpecializationKind() = 1
                  Visiting Decl CXXRecord @0x105047190 
(vector_iterator<int>::vector_iterator) { 
                    "***Implicit declaration ignored***"
                  }
                  Visiting Decl Friend @0x105047350 () { 
                    Visiting Decl ClassTemplate @0x105047310 (vector_iterator) 
{ 
                      Visiting Decl CXXRecord @0x105047280 (vector_iterator) { 
                      }
                      Visiting Decl TemplateTypeParm @0x105047220 
(vector_iterator<int>::<anonymous>) { 
                      }
                      Visiting Decl ClassTemplateSpecialization @0x105046ec0 
(vector_iterator) { 
                        getSpecializationKind() = 1
                        Visiting Decl CXXRecord @0x105047190 
(vector_iterator<int>::vector_iterator) { 
                          "***Implicit declaration ignored***"
                        }
                        Visiting Decl Friend @0x105047350 () { 
                          Visiting Decl ClassTemplate @0x105047310 
(vector_iterator) { 
                            Visiting Decl CXXRecord @0x105047280 
(vector_iterator) { 
                            }
                            Visiting Decl TemplateTypeParm @0x105047220 
(vector_iterator<int>::<anonymous>) { 
                            }
                            Visiting Decl ClassTemplateSpecialization 
@0x105046ec0 (vector_iterator) { 
                              getSpecializationKind() = 1
                              Visiting Decl CXXRecord @0x105047190 
(vector_iterator<int>::vector_iterator) { 
                                "***Implicit declaration ignored***"
                              }
                              Visiting Decl Friend @0x105047350 () { 
                                Visiting Decl ClassTemplate @0x105047310 
(vector_iterator) { 
                                  Visiting Decl CXXRecord @0x105047280 
(vector_iterator) { 
                                  }
                                  Visiting Decl TemplateTypeParm @0x105047220 
(vector_iterator<int>::<anonymous>) { 
                                  }
                                  Visiting Decl ClassTemplateSpecialization 
@0x105046ec0 (vector_iterator) { 
                                    getSpecializationKind() = 1
                                    Visiting Decl CXXRecord @0x105047190 
(vector_iterator<int>::vector_iterator) { 
                                      "***Implicit declaration ignored***"
                                    }
                                    Visiting Decl Friend @0x105047350 () { 
                                      Visiting Decl ClassTemplate @0x105047310 
(vector_iterator) { 
                                        Visiting Decl CXXRecord @0x105047280 
(vector_iterator) { 
                                        }
                                        Visiting Decl TemplateTypeParm 
@0x105047220 (vector_iterator<int>::<anonymous>) { 
                                        }
                                        Visiting Decl 
ClassTemplateSpecialization @0x105046ec0 (vector_iterator) { 
                                          getSpecializationKind() = 1
                                          Visiting Decl CXXRecord @0x105047190 
(vector_iterator<int>::vector_iterator) { 
                                            "***Implicit declaration ignored***"
                                          }
                                          Visiting Decl Friend @0x105047350 () 
{ 
                                            ...etc...
 

Benoit Belley
Sr Principal Developer
M&E-Product Development Group    
 
Autodesk Canada Inc. 
10 Rue Duke
Montreal, Quebec  H3C 2L7 
Canada
 
Direct 514 954-7154
 
 

<<attachment: image002.gif>>

 

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to