[clang-tools-extra] A LibTooling project: Clang-cast

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[clang-tools-extra] A LibTooling project: Clang-cast

Manas via cfe-dev
Hi everyone,

I've always wanted to contribute to Clang, and a few months ago I was looking at the documentations online and found this:

Ideas for new Tools

  • C++ cast conversion tool. Will convert C-style casts ((type) value) to appropriate C++ cast (static_cast, const_cast or reinterpret_cast).

Well, I'm happy to say that after the few months I've finally finished the tool and I'm excited to contribute this back to the community. I also made sure to comply with clang's coding style and quality standards. I would love it if anyone wants to review it/give it a try. Here's my PR on Phabricator: https://reviews.llvm.org/D89765. I will write a .rst in the docs section about how to use in the same PR, but I'm hoping to split the patch into three sections first (strategy to split it is discussed in the link).

Best,
Ray

Addendum (A subset of functionalities)
Checking whether C-style cast was silently performing static/reinterpret_casts/const_cast (or any combination)
const float* const* i;
(int**) i;
error: C style cast can be replaced by 'const_cast<int **>(reinterpret_cast<const int *const *>(i))'
    (int**) i;
    ^~~~~~~~~
    const_cast<int **>(reinterpret_cast<const int *const *>(i))

Checking whether C-style casts were impossible to turn into C++ style casts:
on dependent type T emits the following:
error: C style cast cannot be converted into a C++ style cast
    (T) a;
    ^~~~~

on private inheritance specifier:
struct A {};
class B: A {};
error: C style cast cannot be converted into a C++ style cast
    (B*)(a);
    ^~~~~~~

Checking whether C-style casts are no-ops and can be removed
error: C style cast can be replaced by 'x'
    const int& y = (const int&) x;
                   ^~~~~~~~~~~~~~
                   x

Fix all of the above
note: FIX-IT applied suggested code changes

... except in macros
warning: C style cast can be replaced by 'static_cast<int>(3.5f)' (won't be fixed in macro)
    CC(int, 3.5f);
~~~~^~~~~~~~~~~~

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [clang-tools-extra] A LibTooling project: Clang-cast

Manas via cfe-dev
Hi,

Can I just ask what are the differences between this new tool and the
'cppcoreguidelines-pro-type-cstyle-cast' and 'google-readability-
casting' checks inside clang-tidy?

~Nathan

On Tue, 2020-10-20 at 21:52 -0700, Ray Zhang via cfe-dev wrote:

> Hi everyone,
>
> I've always wanted to contribute to Clang, and a few months ago I was
> looking at the documentations online and found this:
> Ideas for new Tools
> C++ cast conversion tool. Will convert C-style casts ((type) value)
> to appropriate C++ cast (static_cast, const_cast or
> reinterpret_cast).
>
> Well, I'm happy to say that after the few months I've finally
> finished the tool and I'm excited to contribute this back to the
> community. I also made sure to comply with clang's coding style and
> quality standards. I would love it if anyone wants to review it/give
> it a try. Here's my PR on Phabricator:
> https://reviews.llvm.org/D89765. I will write a .rst in the docs
> section about how to use in the same PR, but I'm hoping to split the
> patch into three sections first (strategy to split it is discussed in
> the link).
>
> Best,
> Ray
>
> Addendum (A subset of functionalities)
> Checking whether C-style cast was silently performing
> static/reinterpret_casts/const_cast (or any combination)
> const float* const* i;
> (int**) i;
> error: C style cast can be replaced by 'const_cast<int
> **>(reinterpret_cast<const int *const *>(i))'
>     (int**) i;
>     ^~~~~~~~~
>     const_cast<int **>(reinterpret_cast<const int *const *>(i))
>
> Checking whether C-style casts were impossible to turn into C++ style
> casts:
> on dependent type T emits the following:
> error: C style cast cannot be converted into a C++ style cast
>     (T) a;
>     ^~~~~
>
> on private inheritance specifier:
> struct A {};
> class B: A {};
> error: C style cast cannot be converted into a C++ style cast
>     (B*)(a);
>     ^~~~~~~
>
> Checking whether C-style casts are no-ops and can be removed
> error: C style cast can be replaced by 'x'
>     const int& y = (const int&) x;
>                    ^~~~~~~~~~~~~~
>                    x
>
> Fix all of the above
> note: FIX-IT applied suggested code changes
>
> ... except in macros
> warning: C style cast can be replaced by 'static_cast<int>(3.5f)'
> (won't be fixed in macro)
>     CC(int, 3.5f);
> ~~~~^~~~~~~~~~~~
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [clang-tools-extra] A LibTooling project: Clang-cast

Manas via cfe-dev
I second this. As I've said before and elsewhere this kind of thing is
suitable for clang-tidy.

Thanks,

Stephen

On 21/10/2020 18:04, Nathan James via cfe-dev wrote:

> Hi,
>
> Can I just ask what are the differences between this new tool and the
> 'cppcoreguidelines-pro-type-cstyle-cast' and 'google-readability-
> casting' checks inside clang-tidy?
>
> ~Nathan
>
> On Tue, 2020-10-20 at 21:52 -0700, Ray Zhang via cfe-dev wrote:
>> Hi everyone,
>>
>> I've always wanted to contribute to Clang, and a few months ago I was
>> looking at the documentations online and found this:
>> Ideas for new Tools
>> C++ cast conversion tool. Will convert C-style casts ((type) value)
>> to appropriate C++ cast (static_cast, const_cast or
>> reinterpret_cast).
>>
>> Well, I'm happy to say that after the few months I've finally
>> finished the tool and I'm excited to contribute this back to the
>> community. I also made sure to comply with clang's coding style and
>> quality standards. I would love it if anyone wants to review it/give
>> it a try. Here's my PR on Phabricator:
>> https://reviews.llvm.org/D89765. I will write a .rst in the docs
>> section about how to use in the same PR, but I'm hoping to split the
>> patch into three sections first (strategy to split it is discussed in
>> the link).
>>
>> Best,
>> Ray
>>
>> Addendum (A subset of functionalities)
>> Checking whether C-style cast was silently performing
>> static/reinterpret_casts/const_cast (or any combination)
>> const float* const* i;
>> (int**) i;
>> error: C style cast can be replaced by 'const_cast<int
>> **>(reinterpret_cast<const int *const *>(i))'
>>      (int**) i;
>>      ^~~~~~~~~
>>      const_cast<int **>(reinterpret_cast<const int *const *>(i))
>>
>> Checking whether C-style casts were impossible to turn into C++ style
>> casts:
>> on dependent type T emits the following:
>> error: C style cast cannot be converted into a C++ style cast
>>      (T) a;
>>      ^~~~~
>>
>> on private inheritance specifier:
>> struct A {};
>> class B: A {};
>> error: C style cast cannot be converted into a C++ style cast
>>      (B*)(a);
>>      ^~~~~~~
>>
>> Checking whether C-style casts are no-ops and can be removed
>> error: C style cast can be replaced by 'x'
>>      const int& y = (const int&) x;
>>                     ^~~~~~~~~~~~~~
>>                     x
>>
>> Fix all of the above
>> note: FIX-IT applied suggested code changes
>>
>> ... except in macros
>> warning: C style cast can be replaced by 'static_cast<int>(3.5f)'
>> (won't be fixed in macro)
>>      CC(int, 3.5f);
>> ~~~~^~~~~~~~~~~~
>> _______________________________________________
>> cfe-dev mailing list
>> [hidden email]
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [clang-tools-extra] A LibTooling project: Clang-cast

Manas via cfe-dev
In reply to this post by Manas via cfe-dev
Hi Nathan,

Thanks for bringing these to my attention. I just took a look at these modules and I think currently I have a few things implemented that these don't:

1. This is the main code for google-readability-casting, and it currently actually implements some things incorrectly. One such example is that it doesn't take into account member-pointers during the const cast check. Another example is that for const cast they don't check the other qualifiers (const_cast isn't only for const). There are some things that they also don't provide such as const_cast<...>(static_cast<...>()) nestedness which is specified in the standard as a potential behavior of C-style casts. They also don't take into account that (member) function pointers qualifiers cannot be const-casted away, as according to the standard. They also don't check for clang's behavior when it comes to non-pedantic behavior (unrelated source and destination types for conversions with different qualifiers at the unrelated parts currently only emit a warning in clang). They also don't check for the existence of arrays (partial, fixed size, and VLA's when pedantic is turned off). There are more subtleties like private access specifiers being an extension to static_cast that they don't check for.

2. This is the main code for cppcoreguidelines-pro-type-cstyle-cast, and it has similar parts (some copy pasted to/from the google-readability-casting link), but it's mostly warning for downcasting of class types.

With regards to putting the tool into clang-tidy, I'm actually trying to do that right now and will have a separate patch. I want to keep some of the features (as described in the phab PR) for a standalone application, which is why I opened this patch first.

Best,
Ray

On Wed, Oct 21, 2020 at 10:04 AM Nathan James <[hidden email]> wrote:
Hi,

Can I just ask what are the differences between this new tool and the
'cppcoreguidelines-pro-type-cstyle-cast' and 'google-readability-
casting' checks inside clang-tidy?

~Nathan

On Tue, 2020-10-20 at 21:52 -0700, Ray Zhang via cfe-dev wrote:
> Hi everyone,
>
> I've always wanted to contribute to Clang, and a few months ago I was
> looking at the documentations online and found this:
> Ideas for new Tools
> C++ cast conversion tool. Will convert C-style casts ((type) value)
> to appropriate C++ cast (static_cast, const_cast or
> reinterpret_cast).
>
> Well, I'm happy to say that after the few months I've finally
> finished the tool and I'm excited to contribute this back to the
> community. I also made sure to comply with clang's coding style and
> quality standards. I would love it if anyone wants to review it/give
> it a try. Here's my PR on Phabricator:
> https://reviews.llvm.org/D89765. I will write a .rst in the docs
> section about how to use in the same PR, but I'm hoping to split the
> patch into three sections first (strategy to split it is discussed in
> the link).
>
> Best,
> Ray
>
> Addendum (A subset of functionalities)
> Checking whether C-style cast was silently performing
> static/reinterpret_casts/const_cast (or any combination)
> const float* const* i;
> (int**) i;
> error: C style cast can be replaced by 'const_cast<int
> **>(reinterpret_cast<const int *const *>(i))'
>     (int**) i;
>     ^~~~~~~~~
>     const_cast<int **>(reinterpret_cast<const int *const *>(i))
>
> Checking whether C-style casts were impossible to turn into C++ style
> casts:
> on dependent type T emits the following:
> error: C style cast cannot be converted into a C++ style cast
>     (T) a;
>     ^~~~~
>
> on private inheritance specifier:
> struct A {};
> class B: A {};
> error: C style cast cannot be converted into a C++ style cast
>     (B*)(a);
>     ^~~~~~~
>
> Checking whether C-style casts are no-ops and can be removed
> error: C style cast can be replaced by 'x'
>     const int& y = (const int&) x;
>                    ^~~~~~~~~~~~~~
>                    x
>
> Fix all of the above
> note: FIX-IT applied suggested code changes
>
> ... except in macros
> warning: C style cast can be replaced by 'static_cast<int>(3.5f)'
> (won't be fixed in macro)
>     CC(int, 3.5f);
> ~~~~^~~~~~~~~~~~
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev