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

[Aaron Ballman]

It seems to do the trick nicely -- thanks!

~Aaron

On Tue, Jul 17, 2012 at 4:28 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> 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
>