[cfe-commits] [PATCH] RecursiveASTVisitor.h: Rework r160404, "Eliminating the GCC_CAST hack, take two."

[Douglas Gregor]

LGTM!

Sent from my iPhone

On Nov 18, 2012, at 6:00 AM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:

> Hi doug.gregor,
> 
> RecursiveASTVisitor.h: Rework Doug's r160404, "Eliminating the GCC_CAST hack, take two."
> 
> With this, ARCMT tests would not crash on certain hosts with g++ -O2, eg. cygwin g++-4.5.3.
> 
> r160404 crashed mingw32-g++-4.4.0. I guess method's pointer in conditional expression could not be handled.
> 
> It passes on;
> http://bb.pgr.jp/builders/clang-3stage-x86_64-linux/builds/1094
> http://bb.pgr.jp/builders/cmake-clang-i686-mingw32/builds/3027
> http://bb.pgr.jp/builders/cmake-clang-i686-msvc10/builds/2843
> 
> http://llvm-reviews.chandlerc.com/D125
> 
> Files:
>  clang/include/clang/AST/RecursiveASTVisitor.h
> 
> Index: clang/include/clang/AST/RecursiveASTVisitor.h
> ===================================================================
> --- clang/include/clang/AST/RecursiveASTVisitor.h
> +++ clang/include/clang/AST/RecursiveASTVisitor.h
> @@ -464,20 +464,15 @@
> bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S,
>                                                     bool &EnqueueChildren) {
> 
> -// The cast for DISPATCH_WALK is needed for older versions of g++, but causes
> -// problems for MSVC.  So we'll skip the cast entirely for MSVC.
> -#if defined(_MSC_VER)
> -  #define GCC_CAST(CLASS)
> -#else
> -  #define GCC_CAST(CLASS) (bool (RecursiveASTVisitor::*)(CLASS*))
> -#endif
> -
>   // Dispatch to the corresponding WalkUpFrom* function only if the derived
>   // class didn't override Traverse* (and thus the traversal is trivial).
> #define DISPATCH_WALK(NAME, CLASS, VAR) \
> -  if (&RecursiveASTVisitor::Traverse##NAME == \
> -      GCC_CAST(CLASS)&Derived::Traverse##NAME) \
> -    return getDerived().WalkUpFrom##NAME(static_cast<CLASS*>(VAR)); \
> +  { \
> +    bool (Derived::*DerivedFn)(CLASS*) = &Derived::Traverse##NAME; \
> +    bool (Derived::*BaseFn)(CLASS*) = &RecursiveASTVisitor::Traverse##NAME; \
> +    if (DerivedFn == BaseFn) \
> +      return getDerived().WalkUpFrom##NAME(static_cast<CLASS*>(VAR)); \
> +  } \
>   EnqueueChildren = false; \
>   return getDerived().Traverse##NAME(static_cast<CLASS*>(VAR));
> 
> @@ -516,7 +511,6 @@
>   }
> 
> #undef DISPATCH_WALK
> -#undef GCC_CAST
> 
>   return true;
> }
> <D125.1.patch>