[llvm-commits] [PATCH 1/2] Fix Casting

[dag at cray.com]

David Blaikie <dblaikie at gmail.com> writes:

>>> -  uint8_t *Dst = (uint8_t *)IntVal.getRawData();
>>> +  uint8_t *Dst = reinterpret_cast<uint8_t *>(
>>> +                   const_cast<uint64_t *>(IntVal.getRawData()));

> Especially with const cast changes I'd like to use this as an
> opportunity to consider whether the warning is actually telling us
> something useful, rarther than just something to work around/silence.
>
> Should APInt have a getRawData() non-const function that returns a
> non-const uint64_t*?
> Or should APInt friend this function?
>
> Any alternatives people have in mind?

I found a lot of questionable casting while tracking down these
warnings.  It seems we have a lot of functions that are marked const
that return non-const pointers to internal state.  While such functions
are const in the letter of the law, in my mind they violate the spirit
in the same way a standard library const begin() member returning a
non-const iterator would.

I did not want to tackle such situations because I didn't write the code
originally and I can't properly analyze whether such functions should
become non-const.  I suspect making them non-const is going to turn into
a propagation of many small changes all over the place because that's
how const tends to work.

My goal was to get something that could build with -Werror.  In doing
so, I explicitly used const_const, which should serve as a red flag to
people, hopefully causing the code owners to investigate why such casts
are needed.

I think a re-engineering of the interfaces is a much bigger project and
frankly, I just need to be able to build.

I don't think -Wno-cast-qual is a good idea because we'll just be hiding
these kinds of issues which really should be investigated more
thoroughly.  That's what I hope we can apply the patches and investigate
the uses of const_cast afterward.

                             -David