On Jul 17, 2012, at 5:20 AM, Aaron Ballman <[email protected]> wrote:
> 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! I took a stab at fixing this with r160397. - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
