[cfe-commits] r160281 - /cfe/trunk/include/clang/AST/RecursiveASTVisitor.h

[Douglas Gregor]

On Jul 17, 2012, at 5:20 AM, Aaron Ballman <aaron at aaronballman.com> wrote:

> On Tue, Jul 17, 2012 at 12:26 AM, Douglas Gregor <dgregor at apple.com> wrote:
>> 
>> On Jul 16, 2012, at 8:45 AM, Aaron Ballman <aaron at aaronballman.com> 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