[cfe-commits] [PATCH 11/15] Fix Casting

[dag at cray.com]

Dmitri Gribenko <gribozavr at gmail.com> writes:

>> Here's the kind of thing I'm worrying about:
>>
>> CXCursor cxcursor::MakeCXCursor(const Attr *A, Decl *Parent,
>>                                 CXTranslationUnit TU) {
>>   assert(A && Parent && TU && "Invalid arguments!");
>>   CXCursor C = { GetCursorKind(A), 0, { Parent, const_cast<Attr *>(A), TU } };
>>   return C;
>> }
>>
>> In this case we're const_cast'ing a pointer to a const Attr.  The caller
>> has no idea that the data it sent as an argument can be changed under
>> it.  IMHO that is quite unsafe and should be corrected properly.
>
> It is intrinsic in the AST design that all nodes are semantically
> immutable, but some methods are not marked const for some reason.  But
> even if you have a pointer to a const node somewhere, and need to call
> a non-const method, it is OK cast const away (though ugly).

My point is that a contract exists that is being violated.  To someone
like me, unfamiliar with the code, this looks very scary.  If we mean to
send pointers-to-non-const, we should send pointers-to-non-const or we
should make everything pointers-to-const.  That's the whole point of
const.

At the very least we should clearly document in comments WHY we are
breaking the contract.

But we really should fix the code to be const-correct.

In the meantime, I need to be able to build.  Can I check this in with
the const-cast and code owners can decide what cleanup needs to be done?
People are making changes to these files right now and it's a burden to
keep up and resolve conflicts.

                          -David