LGTM On Jan 18, 2016 12:06 PM, "Vassil Vassilev" <vvasi...@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" <vvasi...@cern.ch> wrote: > >> On 17/01/16 06:34, Douglas Gregor wrote: >> >>> On Jan 16, 2016, at 3:41 PM, Vassil Vassilev <vvasi...@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 >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
