[cfe-dev] Question regarding const-correctness

[Jon Eyolfson]

David Blaikie <dblaikie at gmail.com> writes:

> 2013/11/4 Jon Eyolfson <jon at eyolfson.com>
>
>>
>> Hello Clang Developers,
>>
>> In my brief time with the clang codebase I've noticed instances of
>> const_cast
>> and mutable I don't believe are necessary. Are the uses of const_cast and
>> mutable just temporary to avoid some compile-time type errors?
>
>
> Some might be temporary, there are varying levels of 'technical debt' that
> are acceptable/accrued/historical. Some might be deliberate/acceptable.
> It's hard to say generally.
>
>
>> Does const have
>> a different meaning for internal clang code?
>
>
> Not especially, though there are perhaps some quirks. One is that if an
> class represents immutable objects then we may not bother using 'const' at
> all, since all instances will always be const, we just drop the 'const'
> from all instances. (see the llvm::Type hierarchy for an example of that)

That's a good point, although const wouldn't really hurt either.
>
>
>> Is developer time prioritized
>> elsewhere and you're looking for contributors to help with
>> const-correctness?
>>
>
> Possibly.
>
>
>>
>> The attached example diff demonstrates what I believe is a const-correct
>> version
>> of CFGBlock::getTerminatorCondition(). I don't understand the reasoning
>> behind
>> including a non-const version of this member function that may
>> unintentionally
>> allow bad client code.
>>
>
> What bad client code do you think this might unintentionally allow?

It allows clients to get a Stmt*, which could be used to modify the Stmt
(although looking at the Doxygen, Stmt mostly has const member functions). It
may be an issue if they use children() and modify something they shouldn't.
>
> I believe this code just provided a const and non-const overload for the
> convenience of users (arguable as to whether that's necessary at all, but
> evidently no one relies on it at the moment if your code change compiles).
> It just happens to be easier to implement the overloads by having one
> overload defer to the other.
>
> Personally I probably would've had the non-const overload defer to the
> const overload (and const_cast the result) as that would be more correct
> than possibly casting away const on an actually const object to call a
> non-const member function from a const member function.

What's the reason for the non-const overload at this point? It seems to me that
all AST nodes returned by any analysis should always be const. For me, if I
wrote "Stmt* S = block->getTerminatorCondition();", I'd rather get a compiler
error that it should be "const Stmt* S" then have a const_cast that I personally
don't expect.

Thanks for the reply,
Jon
>
>
>>
>> After applying the diff, clang and the default plugins compile
>> successfully as
>> expected. Granted I don't have any other client code, so I can't see their
>> uses
>> (or misuses). I would argue that bad client code would now get a helpful
>> error
>> message instead of possibly really breaking something in the future.
>> Clients can
>> either fix their code or be explicit about breaking const and include the
>> const_cast themselves.
>>
>> This is my first email to a mailing list, so I apologize in advance for
>> any misuse.
>>
>> Jon
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>
>>