On 26/11/2013 06:29, Sean Silva wrote:



On Tue, Nov 26, 2013 at 12:37 AM, Alp Toker <[email protected] <mailto:[email protected]>> wrote:


    On 22/11/2013 15:54, Ondřej Hošek wrote:

        I've been trying to implement the relevant fix-it, but apart
        from not
        being able to find the end of the expression being cast (to
        insert the
        closing paren), the pathological case

        On Fri, Nov 22, 2013 at 2:13 AM, Dmitri Gribenko
        <[email protected] <mailto:[email protected]>> wrote:

            (for example, static cast of a const cast)

        might pose even more of a challenge, because I have to find
        and print
        the "halfway" type:

        int i = 42;
        const int *cpi = &i;
        float *fi = (float *)cpi;
        // -> reinterpret_cast<float *>(const_cast<int *>(cpi));

        This already spells trouble with regard to code formatting, and it
        gets worse if I have a multi-level pointer type where some of the
        levels are const and some aren't.


    Hi Ondřej,

    I suspect the complexity of converting complex / multi-level
    C-style casts to C++-style casts just isn't worth the effort.


Agreed. This almost borders on a stylistic choice better-suited for clang-tidy.

Sean, you hit the nail on the head and I've reconsidered my suggestions.

The conversion would definitely benefit from the kind of rewrite facilities that already exist in clang-tidy / clang-modernize so it seems better to do there. This would solve difficulties encountered with matching parentheses too. So that's an open task if someone wants to work on it.

In light of this, and considering also that we don't usually make an effort to produce FixIts for warning-level diagnostics in Sema, the original patch to the parser by Ondřej looks fine to me. Dmitri, is that OK with you?

Alp.



-- Sean Silva


    Even if you get it to work, the results wouldn't be pretty. It's
    better to encourage the developer to find an idiomatic solution by
    restructuring their code. Thanks for investigating this though.

    I'd say it's fine just to provide FixIt replacements for the
    common cases like straight const_cast<> that are easy to determine
    using existing CastOperation machinery.



        (Also, I have to move the warning from Parser to Sema because only
        then do I have the type information to generate a fix-it, so I
        don't
        know at all how well it will work with templates.)


    It's also fine to work on this incrementally if you could prepare
    a patch similar to the original one but moving the check to the
    right place in Sema, along with a TODO about the FixIt. (It
    shouldn't be added to Parse because that's a clear dead-end.)

    I don't know what user expectations are for -Wold-style-cast, but
    it might be worth silently permitting the idiomatic C-style
    CK_ToVoid used to suppress unused variable warnings as a special
    case. Do you have an opinion on that?

    Alp.




        Cheers,
        ~~ Ondra
        _______________________________________________
        cfe-commits mailing list
        [email protected] <mailto:[email protected]>
        http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


-- http://www.nuanti.com
    the browser experts


    _______________________________________________
    cfe-commits mailing list
    [email protected] <mailto:[email protected]>
    http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



--
http://www.nuanti.com
the browser experts

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to