Re: r254041 - Teach RAV to pass its DataRecursionQueue to derived classes if they ask for it,

[Richard Smith via cfe-commits]

Hah, looks like a rejects-valid, but it found a real bug, so *shrug*. =)
Hopefully fixed in r254053.

On Tue, Nov 24, 2015 at 5:14 PM, Alexey Samsonov  wrote:

> Hm, looks like we can't compile Clang itself after this change (with GCC):
>
>
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/12237
>
> On Tue, Nov 24, 2015 at 3:50 PM, Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Tue Nov 24 17:50:47 2015
>> New Revision: 254041
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=254041=rev
>> Log:
>> Teach RAV to pass its DataRecursionQueue to derived classes if they ask
>> for it,
>> to allow them to explicitly opt into data recursion despite having
>> overridden
>> Traverse*Stmt or Traverse*Expr. Use this to reintroduce data recursion to
>> the
>> one place that lost it when DataRecursiveASTVisitor was removed.
>>
>> Modified:
>> cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
>> cfe/trunk/tools/libclang/IndexBody.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=254041=254040=254041=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)
>> +++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Tue Nov 24 17:50:47
>> 2015
>> @@ -14,6 +14,8 @@
>>  #ifndef LLVM_CLANG_AST_RECURSIVEASTVISITOR_H
>>  #define LLVM_CLANG_AST_RECURSIVEASTVISITOR_H
>>
>> +#include 
>> +
>>  #include "clang/AST/Attr.h"
>>  #include "clang/AST/Decl.h"
>>  #include "clang/AST/DeclCXX.h"
>> @@ -132,13 +134,13 @@ namespace clang {
>>  /// instantiations will be visited at the same time as the pattern
>>  /// from which they were produced.
>>  template  class RecursiveASTVisitor {
>> +public:
>>/// A queue used for performing data recursion over statements.
>>/// Parameters involving this type are used to implement data
>>/// recursion over Stmts and Exprs within this class, and should
>> -  /// not be explicitly specified by derived classes.
>> +  /// typically not be explicitly specified by derived classes.
>>typedef SmallVectorImpl DataRecursionQueue;
>>
>> -public:
>>/// \brief Return a reference to the derived class.
>>Derived () { return *static_cast(this); }
>>
>> @@ -274,24 +276,32 @@ public:
>>  //  Methods on Stmts 
>>
>>  private:
>> -  template
>> -  struct is_same_member_pointer : std::false_type {};
>> -  template
>> -  struct is_same_member_pointer : std::true_type {};
>> -
>> -  // Traverse the given statement. If the traverse function was not
>> overridden,
>> -  // pass on the data recursion queue information.
>> +  // Determine if the specified derived-class member M can be passed a
>> +  // DataRecursionQueue* argument.
>> +  template
>> +  std::false_type callableWithQueue(...);
>> +  template
>> +  std::true_type callableWithQueue(M m, Derived *d = nullptr, P *p =
>> nullptr,
>> +   DataRecursionQueue *q = nullptr,
>> +   decltype((d->*m)(p, q)) = false);
>> +
>> +  // Traverse the given statement. If the most-derived traverse function
>> takes a
>> +  // data recursion queue, pass it on; otherwise, discard it. Note that
>> the
>> +  // first branch of this conditional must compile whether or not the
>> derived
>> +  // class can take a queue, so if we're taking the second arm, make the
>> first
>> +  // arm call our function rather than the derived class version.
>>  #define TRAVERSE_STMT_BASE(NAME, CLASS, VAR, QUEUE)
>>   \
>> -  (is_same_member_pointer>   \
>> -  ::Traverse##NAME,
>>   \
>> -
>> decltype(::Traverse##NAME),  \
>> -  ::Traverse##NAME>::value
>>\
>> -   ? this->Traverse##NAME(static_cast(VAR), QUEUE)
>>   \
>> +  (decltype(callableWithQueue> *>(::Traverse##NAME))::value   \
>> +   ? static_cast>   \
>> + decltype(
>>\
>> + callableWithQueue> *>(::Traverse##NAME))::value, \
>> + Derived &, RecursiveASTVisitor &>::type>(*this)
>>\
>> + .Traverse##NAME(static_cast(VAR), QUEUE)
>>\
>> : getDerived().Traverse##NAME(static_cast(VAR)))
>>
>> -  // Try to traverse the given statement, or enqueue it if we're
>> performing data
>> -  // recursion in the middle of traversing another statement. Can only
>> be called
>> -  // from within a DEF_TRAVERSE_STMT body or similar context.
>> +// Try to traverse the given statement, or enqueue it if we're
>> performing data
>> +// recursion in the middle of traversing another statement. Can only be
>> called
>> +// from within a DEF_TRAVERSE_STMT body or similar context.
>>  #define TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S)
>>\
>>do {

Re: r254041 - Teach RAV to pass its DataRecursionQueue to derived classes if they ask for it,

[Alexey Samsonov via cfe-commits]

Unfortunately, the bot still seems to be unhappy:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/12246/steps/build%20fresh%20clang/logs/stdio

On Tue, Nov 24, 2015 at 6:45 PM, Richard Smith 
wrote:

> Hah, looks like a rejects-valid, but it found a real bug, so *shrug*. =)
> Hopefully fixed in r254053.
>
>
> On Tue, Nov 24, 2015 at 5:14 PM, Alexey Samsonov 
> wrote:
>
>> Hm, looks like we can't compile Clang itself after this change (with GCC):
>>
>>
>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/12237
>>
>> On Tue, Nov 24, 2015 at 3:50 PM, Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: rsmith
>>> Date: Tue Nov 24 17:50:47 2015
>>> New Revision: 254041
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=254041=rev
>>> Log:
>>> Teach RAV to pass its DataRecursionQueue to derived classes if they ask
>>> for it,
>>> to allow them to explicitly opt into data recursion despite having
>>> overridden
>>> Traverse*Stmt or Traverse*Expr. Use this to reintroduce data recursion
>>> to the
>>> one place that lost it when DataRecursiveASTVisitor was removed.
>>>
>>> Modified:
>>> cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
>>> cfe/trunk/tools/libclang/IndexBody.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=254041=254040=254041=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)
>>> +++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Tue Nov 24
>>> 17:50:47 2015
>>> @@ -14,6 +14,8 @@
>>>  #ifndef LLVM_CLANG_AST_RECURSIVEASTVISITOR_H
>>>  #define LLVM_CLANG_AST_RECURSIVEASTVISITOR_H
>>>
>>> +#include 
>>> +
>>>  #include "clang/AST/Attr.h"
>>>  #include "clang/AST/Decl.h"
>>>  #include "clang/AST/DeclCXX.h"
>>> @@ -132,13 +134,13 @@ namespace clang {
>>>  /// instantiations will be visited at the same time as the pattern
>>>  /// from which they were produced.
>>>  template  class RecursiveASTVisitor {
>>> +public:
>>>/// A queue used for performing data recursion over statements.
>>>/// Parameters involving this type are used to implement data
>>>/// recursion over Stmts and Exprs within this class, and should
>>> -  /// not be explicitly specified by derived classes.
>>> +  /// typically not be explicitly specified by derived classes.
>>>typedef SmallVectorImpl DataRecursionQueue;
>>>
>>> -public:
>>>/// \brief Return a reference to the derived class.
>>>Derived () { return *static_cast(this); }
>>>
>>> @@ -274,24 +276,32 @@ public:
>>>  //  Methods on Stmts 
>>>
>>>  private:
>>> -  template
>>> -  struct is_same_member_pointer : std::false_type {};
>>> -  template
>>> -  struct is_same_member_pointer : std::true_type {};
>>> -
>>> -  // Traverse the given statement. If the traverse function was not
>>> overridden,
>>> -  // pass on the data recursion queue information.
>>> +  // Determine if the specified derived-class member M can be passed a
>>> +  // DataRecursionQueue* argument.
>>> +  template
>>> +  std::false_type callableWithQueue(...);
>>> +  template
>>> +  std::true_type callableWithQueue(M m, Derived *d = nullptr, P *p =
>>> nullptr,
>>> +   DataRecursionQueue *q = nullptr,
>>> +   decltype((d->*m)(p, q)) = false);
>>> +
>>> +  // Traverse the given statement. If the most-derived traverse
>>> function takes a
>>> +  // data recursion queue, pass it on; otherwise, discard it. Note that
>>> the
>>> +  // first branch of this conditional must compile whether or not the
>>> derived
>>> +  // class can take a queue, so if we're taking the second arm, make
>>> the first
>>> +  // arm call our function rather than the derived class version.
>>>  #define TRAVERSE_STMT_BASE(NAME, CLASS, VAR, QUEUE)
>>> \
>>> -  (is_same_member_pointer>> \
>>> -  ::Traverse##NAME,
>>> \
>>> -
>>> decltype(::Traverse##NAME),  \
>>> -  ::Traverse##NAME>::value
>>>\
>>> -   ? this->Traverse##NAME(static_cast(VAR), QUEUE)
>>> \
>>> +  (decltype(callableWithQueue>> *>(::Traverse##NAME))::value   \
>>> +   ? static_cast>> \
>>> + decltype(
>>>\
>>> + callableWithQueue>> *>(::Traverse##NAME))::value, \
>>> + Derived &, RecursiveASTVisitor &>::type>(*this)
>>>\
>>> + .Traverse##NAME(static_cast(VAR), QUEUE)
>>>\
>>> : getDerived().Traverse##NAME(static_cast(VAR)))
>>>
>>> -  // Try to traverse the given statement, or enqueue it if we're
>>> performing data
>>> -  // recursion in the middle of traversing another statement. Can 

r254041 - Teach RAV to pass its DataRecursionQueue to derived classes if they ask for it,

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Nov 24 17:50:47 2015
New Revision: 254041

URL: http://llvm.org/viewvc/llvm-project?rev=254041=rev
Log:
Teach RAV to pass its DataRecursionQueue to derived classes if they ask for it,
to allow them to explicitly opt into data recursion despite having overridden
Traverse*Stmt or Traverse*Expr. Use this to reintroduce data recursion to the
one place that lost it when DataRecursiveASTVisitor was removed.

Modified:
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/tools/libclang/IndexBody.cpp

Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=254041=254040=254041=diff
==
--- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)
+++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Tue Nov 24 17:50:47 2015
@@ -14,6 +14,8 @@
 #ifndef LLVM_CLANG_AST_RECURSIVEASTVISITOR_H
 #define LLVM_CLANG_AST_RECURSIVEASTVISITOR_H
 
+#include 
+
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
@@ -132,13 +134,13 @@ namespace clang {
 /// instantiations will be visited at the same time as the pattern
 /// from which they were produced.
 template  class RecursiveASTVisitor {
+public:
   /// A queue used for performing data recursion over statements.
   /// Parameters involving this type are used to implement data
   /// recursion over Stmts and Exprs within this class, and should
-  /// not be explicitly specified by derived classes.
+  /// typically not be explicitly specified by derived classes.
   typedef SmallVectorImpl DataRecursionQueue;
 
-public:
   /// \brief Return a reference to the derived class.
   Derived () { return *static_cast(this); }
 
@@ -274,24 +276,32 @@ public:
 //  Methods on Stmts 
 
 private:
-  template
-  struct is_same_member_pointer : std::false_type {};
-  template
-  struct is_same_member_pointer : std::true_type {};
-
-  // Traverse the given statement. If the traverse function was not overridden,
-  // pass on the data recursion queue information.
+  // Determine if the specified derived-class member M can be passed a
+  // DataRecursionQueue* argument.
+  template
+  std::false_type callableWithQueue(...);
+  template
+  std::true_type callableWithQueue(M m, Derived *d = nullptr, P *p = nullptr,
+   DataRecursionQueue *q = nullptr,
+   decltype((d->*m)(p, q)) = false);
+
+  // Traverse the given statement. If the most-derived traverse function takes 
a
+  // data recursion queue, pass it on; otherwise, discard it. Note that the
+  // first branch of this conditional must compile whether or not the derived
+  // class can take a queue, so if we're taking the second arm, make the first
+  // arm call our function rather than the derived class version.
 #define TRAVERSE_STMT_BASE(NAME, CLASS, VAR, QUEUE)
\
-  (is_same_member_pointer::value 
\
-   ? this->Traverse##NAME(static_cast(VAR), QUEUE)
\
+  (decltype(callableWithQueue(::Traverse##NAME))::value   
\
+   ? static_cast(::Traverse##NAME))::value, 
\
+ Derived &, RecursiveASTVisitor &>::type>(*this)   
\
+ .Traverse##NAME(static_cast(VAR), QUEUE) 
\
: getDerived().Traverse##NAME(static_cast(VAR)))
 
-  // Try to traverse the given statement, or enqueue it if we're performing 
data
-  // recursion in the middle of traversing another statement. Can only be 
called
-  // from within a DEF_TRAVERSE_STMT body or similar context.
+// Try to traverse the given statement, or enqueue it if we're performing data
+// recursion in the middle of traversing another statement. Can only be called
+// from within a DEF_TRAVERSE_STMT body or similar context.
 #define TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S) 
\
   do { 
\
 if (!TRAVERSE_STMT_BASE(Stmt, Stmt, S, Queue)) 
\
@@ -535,14 +545,6 @@ bool RecursiveASTVisitor::Trave
   if (!S)
 return true;
 
-  // If TraverseStmt was overridden (and called the base class version), don't
-  // do any data recursion; it would be observable.
-  if (!is_same_member_pointer::value)
-return dataTraverseNode(S, nullptr);
-
   if (Queue) {
 Queue->push_back(S);
 return true;

Modified: