Skip to content

Commit 1b82675

Browse files
authored
Fix #14317 Syntax error using macro inside ifdef block (danmar#8123)
Changed so that auto-configured values (without specified values) are set to itself instead of 1. From [issue 14317](https://trac.cppcheck.net/ticket/14317) [](url) isless=islesss instead of isless=1
1 parent c4f754e commit 1b82675

File tree

4 files changed

+143
-51
lines changed

4 files changed

+143
-51
lines changed

lib/preprocessor.cpp

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -428,12 +428,15 @@ static std::string readcondition(const simplecpp::Token *iftok, const std::set<s
428428
}
429429

430430
std::set<std::string> configset;
431+
bool isNotDefinedMacro = false;
431432
for (; sameline(iftok,cond); cond = cond->next) {
432433
if (cond->op == '!') {
433434
if (!sameline(iftok,cond->next) || !cond->next->name)
434435
break;
435-
if (cond->next->str() == "defined")
436+
if (cond->next->str() == "defined") {
437+
isNotDefinedMacro = true;
436438
continue;
439+
}
437440
configset.insert(cond->next->str() + "=0");
438441
continue;
439442
}
@@ -444,8 +447,15 @@ static std::string readcondition(const simplecpp::Token *iftok, const std::set<s
444447
break;
445448
if (dtok->op == '(')
446449
dtok = dtok->next;
447-
if (sameline(iftok,dtok) && dtok->name && defined.find(dtok->str()) == defined.end() && undefined.find(dtok->str()) == undefined.end())
448-
configset.insert(dtok->str());
450+
451+
if (sameline(iftok,dtok) && dtok->name && defined.find(dtok->str()) == defined.end() && undefined.find(dtok->str()) == undefined.end()) {
452+
if (!isNotDefinedMacro) {
453+
configset.insert(dtok->str() + "=" + dtok->str()); // if defined is set to itself.
454+
} else {
455+
configset.insert(dtok->str());
456+
}
457+
}
458+
isNotDefinedMacro = false;
449459
}
450460
std::string cfgStr;
451461
for (const std::string &s : configset) {
@@ -463,11 +473,12 @@ static bool hasDefine(const std::string &userDefines, const std::string &cfg)
463473
}
464474

465475
std::string::size_type pos = 0;
476+
const std::string cfgname = cfg.substr(0, cfg.find('='));
466477
while (pos < userDefines.size()) {
467-
pos = userDefines.find(cfg, pos);
478+
pos = userDefines.find(cfgname, pos);
468479
if (pos == std::string::npos)
469480
break;
470-
const std::string::size_type pos2 = pos + cfg.size();
481+
const std::string::size_type pos2 = pos + cfgname.size();
471482
if ((pos == 0 || userDefines[pos-1U] == ';') && (pos2 == userDefines.size() || userDefines[pos2] == '='))
472483
return true;
473484
pos = pos2;
@@ -553,8 +564,11 @@ static void getConfigs(const simplecpp::TokenList &tokens, std::set<std::string>
553564
const simplecpp::Token *expr1 = cmdtok->next;
554565
if (sameline(tok,expr1) && expr1->name && !sameline(tok,expr1->next))
555566
config = expr1->str();
556-
if (defined.find(config) != defined.end())
567+
if (defined.find(config) != defined.end()) {
557568
config.clear();
569+
} else if ((cmdtok->str() == "ifdef") && sameline(cmdtok,expr1) && !config.empty()) {
570+
config.append("=" + expr1->str()); //Set equal to itself if ifdef.
571+
}
558572
} else if (cmdtok->str() == "if") {
559573
config = readcondition(cmdtok, defined, undefined);
560574
}
@@ -594,6 +608,24 @@ static void getConfigs(const simplecpp::TokenList &tokens, std::set<std::string>
594608
}
595609
}
596610

611+
// check if config already exists in the ret set, but as a more general or more specific version
612+
if (cmdtok->str() != "ifndef")
613+
{
614+
const std::string::size_type eq = config.find('=');
615+
const std::string config2 = (eq != std::string::npos) ? config.substr(0, eq) : config + "=" + config;
616+
const std::set<std::string>::iterator it2 = ret.find(config2);
617+
if (it2 != ret.end()) {
618+
if (eq == std::string::npos) {
619+
// The instance in ret is more specific than the one in config (no =value), replace it with the one in config
620+
ret.erase(it2);
621+
} else {
622+
// The instance in ret is more general than the one in config (have =value), keep the one in ret
623+
config.clear();
624+
continue;
625+
}
626+
}
627+
}
628+
597629
configs_if.push_back((cmdtok->str() == "ifndef") ? std::string() : config);
598630
configs_ifndef.push_back((cmdtok->str() == "ifndef") ? std::move(config) : std::string());
599631
ret.insert(cfg(configs_if,userDefines));
@@ -627,8 +659,18 @@ static void getConfigs(const simplecpp::TokenList &tokens, std::set<std::string>
627659
configs_if.push_back(std::move(config));
628660
ret.insert(cfg(configs_if, userDefines));
629661
} else if (!configs_ifndef.empty()) {
630-
configs_if.push_back(configs_ifndef.back());
631-
ret.insert(cfg(configs_if, userDefines));
662+
//Check if ifndef already existing in ret as more general/specific version
663+
const std::string &confCandidate = configs_ifndef.back();
664+
if (ret.find(confCandidate) == ret.end()) {
665+
// No instance of config_ifndef in ret. Check if a more specific version exists, in that case replace it
666+
const std::set<std::string>::iterator it = ret.find(confCandidate + "=" + confCandidate);
667+
if (it != ret.end()) {
668+
// The instance in ret is more specific than the one in confCandidate (no =value), replace it with the one in confCandidate
669+
ret.erase(it);
670+
}
671+
configs_if.push_back(configs_ifndef.back());
672+
ret.insert(cfg(configs_if, userDefines));
673+
}
632674
}
633675
} else if (cmdtok->str() == "endif" && !sameline(tok, cmdtok->next)) {
634676
if (!configs_if.empty())

test/cli/helloworld_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ def test_addon_relative_path():
107107
filename = os.path.join('helloworld', 'main.c')
108108
assert ret == 0, stdout
109109
assert stdout == ('Checking %s ...\n'
110-
'Checking %s: SOME_CONFIG...\n' % (filename, filename))
110+
'Checking %s: SOME_CONFIG=SOME_CONFIG...\n' % (filename, filename))
111111
assert stderr == ('[%s:5]: (error) Division by zero.\n'
112112
'[%s:4]: (style) misra violation (use --rule-texts=<file> to get proper output)\n' % (filename, filename))
113113

@@ -125,7 +125,7 @@ def test_addon_with_gui_project(tmp_path):
125125
assert ret == 0, stdout
126126
assert stdout.strip().split('\n') == [
127127
'Checking %s ...' % filename,
128-
'Checking %s: SOME_CONFIG...' % filename
128+
'Checking %s: SOME_CONFIG=SOME_CONFIG...' % filename
129129
]
130130
assert stderr == ('[%s:5]: (error) Division by zero.\n'
131131
'[%s:4]: (style) misra violation (use --rule-texts=<file> to get proper output)\n' % (filename, filename))

test/testcppcheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,7 @@ class TestCppcheck : public TestFixture {
618618
// the internal errorlist is cleared after each check() call
619619
ASSERT_EQUALS(1, errorLogger.errmsgs.size());
620620
auto it = errorLogger.errmsgs.cbegin();
621-
ASSERT_EQUALS("test.cpp:0:0: information: The configuration 'X' was not checked because its code equals another one. [purgedConfiguration]",
621+
ASSERT_EQUALS("test.cpp:0:0: information: The configuration 'X=X' was not checked because its code equals another one. [purgedConfiguration]",
622622
it->toString(false, templateFormat, ""));
623623
}
624624

0 commit comments

Comments
 (0)