On Tue, Jul 17, 2012 at 12:26 AM, Douglas Gregor <[email protected]> wrote: > > On Jul 16, 2012, at 8:45 AM, Aaron Ballman <[email protected]> wrote: > >> Author: aaronballman >> Date: Mon Jul 16 10:45:33 2012 >> New Revision: 160281 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=160281&view=rev >> Log: >> Fixing an MSVC warning -- the compiler did not like the cast added to work >> around a g++ bug (it would claim to possibly emit incorrect code). >> >> Modified: >> cfe/trunk/include/clang/AST/RecursiveASTVisitor.h >> >> Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=160281&r1=160280&r2=160281&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original) >> +++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Mon Jul 16 10:45:33 >> 2012 >> @@ -464,12 +464,19 @@ >> 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). >> - // The cast here is necessary to work around a bug in old versions of g++. >> #define DISPATCH_WALK(NAME, CLASS, VAR) \ >> if (&RecursiveASTVisitor::Traverse##NAME == \ >> - (bool (RecursiveASTVisitor::*)(CLASS*))&Derived::Traverse##NAME) \ >> + GCC_CAST(CLASS)&Derived::Traverse##NAME) \ >> return getDerived().WalkUpFrom##NAME(static_cast<CLASS*>(VAR)); \ >> EnqueueChildren = false; \ >> return getDerived().Traverse##NAME(static_cast<CLASS*>(VAR)); >> @@ -509,6 +516,7 @@ >> } > > This GCC_CAST thing is pretty hideous. Is there no non-macro, well-formed way > to appease both compilers? Like, for example, creating a local variable of > type bool (recursiveASTVisitor::*)(CLASS*)?
I agree that it's not the prettiest solution, but I've not found a way that I'm comfortable with to appease both compilers. I can test MSVC easily enough, but I have no idea what older version of g++ was exhibiting the behavior requiring the cast in the first place, or how the bug manifested itself there. So I'm worried about regressing the issue unintentionally. At least this way, the change has minimal impact while solving the problem (even if it's pretty ugly). I'm open to suggestions on how to verify other changes though! ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
