cppcheck icon indicating copy to clipboard operation
cppcheck copied to clipboard

avoid some `const_cast` usage

Open firewave opened this issue 3 years ago • 13 comments

firewave avatar Aug 07 '22 16:08 firewave

doesn't static_cast mean that the type might change so it's less safe?

danmar avatar Aug 07 '22 17:08 danmar

doesn't static_cast mean that the type might change so it's less safe?

I don't know. I was surprised that const_cast<const T*> is actually working.

firewave avatar Aug 07 '22 17:08 firewave

I do not feel our code is super elegant. An alternative would be to create a common private const function that is called both from the const and non-const method. We don't need to explicitly cast at all then.

danmar avatar Aug 07 '22 17:08 danmar

I do not feel our code is super elegant. An alternative would be to create a common private const function that is called both from the const and non-const method. We don't need to explicitly cast at all then.

I am looking into this (also as preparation for misc-const-correctness) and in some cases the non-const pointers might not be necessary at all which might clean up some things.

firewave avatar Aug 07 '22 18:08 firewave

I am looking into this (also as preparation for misc-const-correctness) and in some cases the non-const pointers might not be necessary at all which might clean up some things.

ok I close this PR for now.. but feel free to clean up the code..

danmar avatar Aug 21 '22 15:08 danmar

ok I close this PR for now.. but feel free to clean up the code..

This would require introducing additional private *Internal() members which would not clean up the code much.

This PR still gets rid of unnecessary/incorrected const_cast calls though.

firewave avatar Dec 11 '22 20:12 firewave

I feel that static_cast is even worse than const_cast though. const_cast is more strict.

danmar avatar Dec 18 '22 18:12 danmar

const_cast is for casting away const. In this case we are just changing the type without touching the const thus making it unnecessary.

firewave avatar Dec 18 '22 18:12 firewave

const_cast is for casting away const. In this case we are just changing the type without touching the const thus making it unnecessary.

If the casts would change the type then const_cast would generate a compiler error right? So it seems it just casts away the constness.

danmar avatar Dec 18 '22 18:12 danmar

If the casts would change the type then const_cast would generate a compiler error right? So it seems it just casts away the constness.

I assumed that const_cast is only to get rid of const and not more restrictive than other casts. Need to do some tests with that.

firewave avatar Dec 18 '22 19:12 firewave

Aren't those instances the canonical use case for const_cast, i.e. resolving between const/non-const overloads?

chrchr-github avatar Sep 28 '23 08:09 chrchr-github

Partially superseded by #5720. Still lacks the SymbolDatabase adjustments. Even after that I would like to keep it open until the static_cast question has been answered.

firewave avatar Dec 02 '23 13:12 firewave

#5720 also includes SymbolDatabase now - so this PR is complete obsolete.

firewave avatar Dec 02 '23 14:12 firewave