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

[Douglas Gregor]

On Jul 17, 2012, at 3:23 PM, Aaron Ballman <aaron at aaronballman.com> wrote:

> It seems to do the trick nicely -- thanks!

It actually just triggered a failure on one of the buildbots :(

	http://lab.llvm.org:8011/builders/clang-x86_64-darwin11-self-mingw32/builds/5350/steps/compile/logs/stdio

Looking into it.

	- Doug

> ~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
>>