[PATCH][Modules][PR26179]

[Richard Smith via cfe-commits]

LGTM
On Jan 18, 2016 12:06 PM, "Vassil Vassilev" <vvasilev at cern.ch> wrote:

> Attaching v3 of the patch. Added your case to the current test and fixed
> my silly non-array mistake.
> -- Vassil
> On 18/01/16 20:38, Richard Smith wrote:
>
> Please also add a test case that your old patch would have failed on, such
> as:
>
> m1.h:
> extern int a[];
>
> m2.h:
> extern int a[5];
>
> x.cc:
> #include "m1.h"
> #include "m2.h"
> int *p = a;
> On Jan 18, 2016 9:28 AM, "Vassil Vassilev" <vvasilev at cern.ch> wrote:
>
>> On 17/01/16 06:34, Douglas Gregor wrote:
>>
>>> On Jan 16, 2016, at 3:41 PM, Vassil Vassilev <vvasilev at cern.ch> wrote:
>>>>
>>>> Hi,
>>>>   Could somebody review the attached patch. It fixes
>>>> https://llvm.org/bugs/show_bug.cgi?id=26179
>>>> Many thanks!
>>>> Vassil
>>>> <0001-modules-Teach-clang-to-how-to-merge-variable-redecls.patch>
>>>>
>>>
>>> +      // We can get decls with different types on the redecl chain. Eg.
>>> +      // template <typename T> struct S { static T Var[]; }; // #1
>>> +      // template <typename T> T S<T>::Var[sizeof(T)]; // #2
>>> +      // Trying to compare #1 and #2 should go through their canonical
>>> decls.
>>> +      QualType VarXTy = VarX->getCanonicalDecl()->getType();
>>> +      QualType VarYTy = VarY->getCanonicalDecl()->getType();
>>> +      if (Context.hasSameType(VarXTy, VarYTy))
>>> +        return true;
>>>
>>> Completing an incomplete array is (I think) the only case in which this
>>> can happen. How about checking for that case specifically (i.e., it’s okay
>>> to have one be an incomplete array and the other to be any other kind of
>>> array with the same element type), rather than a blanket check on the
>>> canonical declaration types?
>>>
>>>         - Doug
>>>
>>> Thanks for the comments. Patch v2 attached.
>> -- Vassil
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160118/beaf4de1/attachment-0001.html>