walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix #9647: Set correct enum value (#2856) 

* Tokenize: Set varId for variables in enum

Set varIds in enum values. It was previously disabled in 5119ae84b8
to avoid issues with enums named the same as global variables. Take care
to only set varids to variables used to set the value of an enumerator,
not the enumerator itself. This is somewhat complicated by the fact that
at the time this happens, astOperand1(), astOperand2(), astParent() etc
are not set. The current implementation is not perfect, for example in
the code below, y will not have a varid set, but x and z will. This is
deemed sufficient for now.

            int x, y, z;
            enum E { a = f(x, y, z); };

* Fix #9647: Value of enums with variables as init values

C++ allows enum values to be set using constexprs, which cppcheck did
not handle before. To solve this, add a new pass to valueflow to update
enum values after global consts have been processed. In order to do so,
I moved all settings of enum values to valueflow. After setting the enum
values, we need another call to valueFlowNumber() to actually set users
of the enums.

There is still room for improvements, since each pass of
valueFlowGlobalConstVar() and valueFlowEnumValue() only sets variables
that are possible to set directly, and not if setting the value of a
variable allows us to set the value of another. For example

	constexpr int a = 5;
	constexpr int b = a + 5;
	enum E { X = a };
	constexpr E e = X;

Here both b and e will not have their values set, even though cppcheck
should be possible to figure out their values. That's for another PR
though.

This was tested by running test-my-pr.py with 500 packages. The only
difference was one error message in fairy-stockfish_11.1, where cppcheck
now printed the correct size of an array instead of 2147483648 which I
assume is some kind of default value. In that package, using a constexpr
when setting enum values is common, but as mentioned, there was no
change in the number of warnings.
This commit is contained in:
Rikard Falkeborn 2020-10-22 07:45:04 +02:00 committed by GitHub
parent 64638d82bb
commit d7a8e25d92
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 116 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
lib/symboldatabase.cpp
View File

@ -1272,14 +1272,10 @@ void SymbolDatabase::createSymbolDatabaseEnums()
const_cast<Token *>(i.name)->enumerator(&i);
}
// fill in enumerator values
for (std::list<Scope>::iterator it = scopeList.begin(); it != scopeList.end(); ++it) {
if (it->type != Scope::eEnum)
continue;
MathLib::bigint value = 0;
bool prev_enum_is_known = true;
for (Enumerator & enumerator : it->enumeratorList) {
// look for initialization tokens that can be converted to enumerators and convert them
if (enumerator.start) {
@ -1292,28 +1288,6 @@ void SymbolDatabase::createSymbolDatabaseEnums()
const_cast<Token *>(tok3)->enumerator(e);
}
}
// look for possible constant folding expressions
// rhs of operator:
Token *rhs = enumerator.start->previous()->astOperand2();
// constant folding of expression:
ValueFlow::valueFlowConstantFoldAST(rhs, mSettings);
// get constant folded value:
if (rhs && rhs->hasKnownIntValue()) {
enumerator.value = rhs->values().front().intvalue;
enumerator.value_known = true;
value = enumerator.value + 1;
prev_enum_is_known = true;
} else
prev_enum_is_known = false;
}
// not initialized so use default value
else if (prev_enum_is_known) {
enumerator.value = value++;
enumerator.value_known = true;
}
}
}

2
lib/tokenize.cpp
View File

@ -3605,7 +3605,7 @@ void Tokenizer::setVarIdPass1()
continue;
}
if (!scopeStack.top().isEnum) {
if (!scopeStack.top().isEnum || !(Token::Match(tok->previous(), "{|,") && Token::Match(tok->next(), ",|=|}"))) {
const std::map<std::string, int>::const_iterator it = variableMap.find(tok->str());
if (it != variableMap.end()) {
tok->varId(it->second);

31
lib/valueflow.cpp
View File

@ -1628,6 +1628,34 @@ static void valueFlowOppositeCondition(SymbolDatabase *symboldatabase, const Set
}
}
static void valueFlowEnumValue(SymbolDatabase * symboldatabase, const Settings * settings)
{
for (Scope & scope : symboldatabase->scopeList) {
if (scope.type != Scope::eEnum)
continue;
MathLib::bigint value = 0;
bool prev_enum_is_known = true;
for (Enumerator & enumerator : scope.enumeratorList) {
if (enumerator.start) {
Token *rhs = enumerator.start->previous()->astOperand2();
ValueFlow::valueFlowConstantFoldAST(rhs, settings);
if (rhs && rhs->hasKnownIntValue()) {
enumerator.value = rhs->values().front().intvalue;
enumerator.value_known = true;
value = enumerator.value + 1;
prev_enum_is_known = true;
} else
prev_enum_is_known = false;
} else if (prev_enum_is_known) {
enumerator.value = value++;
enumerator.value_known = true;
}
}
}
}
static void valueFlowGlobalConstVar(TokenList* tokenList, const Settings *settings)
{
// Get variable values...
@ -6791,11 +6819,14 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
for (Token *tok = tokenlist->front(); tok; tok = tok->next())
tok->clearValueFlow();
valueFlowEnumValue(symboldatabase, settings);
valueFlowNumber(tokenlist);
valueFlowString(tokenlist);
valueFlowArray(tokenlist);
valueFlowUnknownFunctionReturn(tokenlist, settings);
valueFlowGlobalConstVar(tokenlist, settings);
valueFlowEnumValue(symboldatabase, settings);
valueFlowNumber(tokenlist);
valueFlowGlobalStaticVar(tokenlist, settings);
valueFlowPointerAlias(tokenlist);
valueFlowLifetime(tokenlist, symboldatabase, errorLogger, settings);

14
test/testsymboldatabase.cpp
View File

@ -337,6 +337,7 @@ private:
TEST_CASE(enum6);
TEST_CASE(enum7);
TEST_CASE(enum8);
TEST_CASE(enum9);
TEST_CASE(sizeOfType);
@ -5034,6 +5035,19 @@ private:
ASSERT(!X5->value_known);
}
void enum9() {
GET_SYMBOL_DB("const int x = 7; enum E { X0 = x, X1 };\n");
ASSERT(db != nullptr);
const Enumerator *X0 = db->scopeList.back().findEnumerator("X0");
ASSERT(X0);
ASSERT(X0->value_known);
ASSERT_EQUALS(X0->value, 7);
const Enumerator *X1 = db->scopeList.back().findEnumerator("X1");
ASSERT(X1);
ASSERT(X1->value_known);
ASSERT_EQUALS(X1->value, 8);
}
void sizeOfType() {
// #7615 - crash in Symboldatabase::sizeOfType()
GET_SYMBOL_DB("enum e;\n"

70
test/testvarid.cpp
View File

@ -194,6 +194,12 @@ private:
TEST_CASE(varidclass20); // #7578: int (*p)[2]
TEST_CASE(varid_classnameshaddowsvariablename); // #3990
TEST_CASE(varidenum1);
TEST_CASE(varidenum2);
TEST_CASE(varidenum3);
TEST_CASE(varidenum4);
TEST_CASE(varidenum5);
TEST_CASE(varidnamespace1);
TEST_CASE(varidnamespace2);
TEST_CASE(usingNamespace1);
@ -3084,6 +3090,70 @@ private:
ASSERT_EQUALS(expected, tokenize(code));
}
void varidenum1() {
const char code[] = "const int eStart = 6;\n"
"enum myEnum {\n"
" A = eStart;\n"
"};\n";
const char expected[] = "1: const int eStart@1 = 6 ;\n"
"2: enum myEnum {\n"
"3: A = eStart@1 ;\n"
"4: } ;\n";
ASSERT_EQUALS(expected, tokenize(code));
}
void varidenum2() {
const char code[] = "const int eStart = 6;\n"
"enum myEnum {\n"
" A = f(eStart);\n"
"};\n";
const char expected[] = "1: const int eStart@1 = 6 ;\n"
"2: enum myEnum {\n"
"3: A = f ( eStart@1 ) ;\n"
"4: } ;\n";
ASSERT_EQUALS(expected, tokenize(code));
}
void varidenum3() {
const char code[] = "const int eStart = 6;\n"
"enum myEnum {\n"
" A = f(eStart, x);\n"
"};\n";
const char expected[] = "1: const int eStart@1 = 6 ;\n"
"2: enum myEnum {\n"
"3: A = f ( eStart@1 , x ) ;\n"
"4: } ;\n";
ASSERT_EQUALS(expected, tokenize(code));
}
void varidenum4() {
const char code[] = "const int eStart = 6;\n"
"enum myEnum {\n"
" A = f(x, eStart);\n"
"};\n";
const char expected[] = "1: const int eStart@1 = 6 ;\n"
"2: enum myEnum {\n"
"3: A = f ( x , eStart@1 ) ;\n"
"4: } ;\n";
ASSERT_EQUALS(expected, tokenize(code));
}
void varidenum5() {
const char code[] = "const int eStart = 6;\n"
"enum myEnum {\n"
" A = f(x, eStart, y);\n"
"};\n";
const char expected[] = "1: const int eStart@1 = 6 ;\n"
"2: enum myEnum {\n"
"3: A = f ( x , eStart@1 , y ) ;\n"
"4: } ;\n";
const char current[] = "1: const int eStart@1 = 6 ;\n"
"2: enum myEnum {\n"
"3: A = f ( x , eStart , y ) ;\n"
"4: } ;\n";
TODO_ASSERT_EQUALS(expected, current, tokenize(code));
}
void varid_classnameshaddowsvariablename() {
const char code[] = "class Data;\n"
"void strange_declarated(const Data& Data);\n"