Re: [PATCH][Modules][PR26179]

[Vassil Vassilev via cfe-commits]

Thanks!
--Vassil
On 22/01/16 20:07, Yaron Keren wrote:

Committed revision 258524.

2016-01-21 19:33 GMT+02:00 Vassil Vassilev via cfe-commits <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>>:

    ping... (perhaps someone else with commit rights can check it it...)
    --Vassil

    On 18/01/16 22:07, Vassil Vassilev wrote:

    Thanks! Could you check it in?
    -- Vassil
    On 18/01/16 21:38, Richard Smith via cfe-commits wrote:

    LGTM

    On Jan 18, 2016 12:06 PM, "Vassil Vassilev" <vvasi...@cern.ch
    <mailto: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 <mailto: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 <mailto: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 <mailto:cfe-commits@lists.llvm.org>
    http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

    _______________________________________________
    cfe-commits mailing list
    cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>
    http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits