Stl: src: Change C casts to C++ casts

Created on 18 Oct 2019  路  6Comments  路  Source: microsoft/STL

We've cleaned up the inc headers to always use C++ casts. We need to do the same for the src source files.

enhancement

Most helpful comment

Clang is supported for inc, but not for src. (It can't be in general due to the /clr support machinery, which is not yet built by our CMake build system.) However, attempting to build src with Clang -Wold-style-cast is a good idea.

All 6 comments

I would like to take on this. I don't see in the repo history a commit or series of commits for this refactoring you mentioned - can you point me to it?

Thank you! Unfortunately, we aren鈥檛 able to make our pre-GitHub commit history public, but the header changes were simply performed by grepping for C cast syntax, and replacing each occurrence with static_cast, const_cast, and/or reinterpret_cast as described by https://eel.is/c++draft/expr.cast#4 . (Careful attention is occasionally required, e.g. to avoid using reinterpret_cast when static_cast suffices.) In some cases, we changed the code to avoid the need for any casting.

(Standardese citations as of WG21-N4835.)

Oh, I figured. Thanks for clarifying.

the header changes were simply performed by grepping for C cast syntax

Pretty much what I've been doing. If only we could run g++ -Wold-style-cast on it. :)

Ok, will report later. For this issue, a single commit with all of the changes should be alright, correct?

Yes, with commit notes explaining (with line numbers) any unusual replacements. If you want, you could arrange a series of commits (e.g. all const_casts, then all static_casts, etc.), although that鈥檚 not necessary. Thanks! 馃樃

If only we could run g++ -Wold-style-cast on it. :)

Clang has -Wold-style-cast, and it's a supported front-end.

Clang is supported for inc, but not for src. (It can't be in general due to the /clr support machinery, which is not yet built by our CMake build system.) However, attempting to build src with Clang -Wold-style-cast is a good idea.

Was this page helpful?
0 / 5 - 0 ratings