const_cast

[Ken-Patrick Lehrmann]

Hi,
in the code of cppcheck, I see some usage of const_cast to remove const qualifier. For instance, there are some functions like

void foo (const A*a){
  const_cast<A *>(a)->modifyA(42); // but foo is not supposed to change a ?!?
}

They seem very wrong to me (https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-casts-const). Am I missing something ? Is there any reason for those ?

I'll probably try to fix those if it is ok for everyone.

[Paul Fultz]

I think that would be great, but I am not sure easy it would be to change.

[Daniel Marjamäki]

Feel free to change 1 or 2 of those and then show us how you are solving these.. if such cast can be removed cleanly then that is good.

[Robert Reif]

The checks should only deal with a const version of the token list and tokens. They should not be allowed to modify the internal state of the tokenizer and symbol database.

The tokenizer, symbol database and template simplifier need to edit the internal state but that can be very hard because they have pointers to the internal states (tokens) and changing it invalidates these pointers. We could have private non-const versions of the const functions and use friend to give access.

[Ken-Patrick Lehrmann]

Thanks for the feedback.
I had a quick look, and indeed, some const_cast are rather easy to remove, and some are very complex.
The easy one are due to a missing non const override of some methods, for instance:

diff --git a/lib/token.h b/lib/token.h
index a9a0b7b6e..dae71f899 100644
--- a/lib/token.h
+++ b/lib/token.h
@@ -1088,12 +1088,21 @@ public:
     void astOperand1(Token *tok);
     void astOperand2(Token *tok);

+    Token * astOperand1() {
+        return mImpl->mAstOperand1;
+    }
     const Token * astOperand1() const {
         return mImpl->mAstOperand1;
     }
+    Token * astOperand2() {
+        return mImpl->mAstOperand2;
+    }
     const Token * astOperand2() const {
         return mImpl->mAstOperand2;
     }
+    Token * astParent() {
+        return mImpl->mAstParent;
+    }
     const Token * astParent() const {
         return mImpl->mAstParent;
     }
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 2856e8849..638993fdb 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -3714,22 +3714,22 @@ struct ValueFlowConditionHandler {
                 }

                 if (Token::Match(tok->astParent(), "%oror%|&&")) {
-                    Token *parent = const_cast<Token *>(tok->astParent());
+                    Token *parent = tok->astParent();
                     const std::string &op(parent->str());

                     if (parent->astOperand1() == tok && ((op == "&&" && Token::Match(tok, "==|>=|<=|!")) ||
                                                          (op == "||" && Token::Match(tok, "%name%|!=")))) {
-                        for (; parent && parent->str() == op; parent = const_cast<Token *>(parent->astParent())) {
+                        for (; parent && parent->str() == op; parent = parent->astParent()) {
                             std::stack<Token *> tokens;
-                            tokens.push(const_cast<Token *>(parent->astOperand2()));
+                            tokens.push(parent->astOperand2());
                             bool assign = false;
                             while (!tokens.empty()) {
                                 Token *rhstok = tokens.top();
                                 tokens.pop();
                                 if (!rhstok)
                                     continue;
-                                tokens.push(const_cast<Token *>(rhstok->astOperand1()));
-                                tokens.push(const_cast<Token *>(rhstok->astOperand2()));
+                                tokens.push(rhstok->astOperand1());
+                                tokens.push(rhstok->astOperand2());
                                 if (rhstok->varId() == varid)
                                     setTokenValue(rhstok, cond.true_values.front(), settings);
                                 else if (Token::Match(rhstok, "++|--|=") &&
@@ -3741,7 +3741,7 @@ struct ValueFlowConditionHandler {
                             if (assign)
                                 break;
                             while (parent->astParent() && parent == parent->astParent()->astOperand2())
-                                parent = const_cast<Token *>(parent->astParent());
+                                parent = parent->astParent();
                         }
                     }
                 }

[Daniel Marjamäki]

That fix seems acceptable. I would like that you create a github pull request for that one.

[Ken-Patrick Lehrmann]

Sure.
I'll probably remove a bit more const_cast before submitting the pull-request.

[Ken-Patrick Lehrmann]

https://github.com/danmar/cppcheck/pull/1886