Skip to content

Fix #722 (#if: &&, || and ?: do not short-circuit) #723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danmar merged 3 commits into from
Nov 7, 2026
Merged

Fix #722 (#if: &&, || and ?: do not short-circuit) #723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17f8cf8
Fix #722 (#if: &&, || and ?: do not short-circuit)
dmcppcheck Nov 6, 2026
1f255d2
Fold chained ?: in linear time
dmcppcheck Nov 7, 2026
a3bd7a1
#if: and, or, bitand, bitor and xor short-circuit like &&, ||, &, | a…
dmcppcheck Nov 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 135 additions & 56 deletions simplecpp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,31 @@ static bool isValidPunctuatorPaste(const std::string &s)
return isPunctuator(s) || (s.size() >= 2 && s[0] == '.' && std::isdigit(static_cast<unsigned char>(s[1])));
}

/*
* When an operation can't be evaluated (division by zero), its result is an
* error value. The error is only reported if the error value is used, so an
* operand that is not evaluated, as in "0 && (1/0)", does not cause an error.
*/
static const std::string ERROR_VALUE("#error:");

static bool isErrorValue(const simplecpp::Token *tok)
{
return tok && tok->str().compare(0, ERROR_VALUE.size(), ERROR_VALUE) == 0;
}

static bool isValue(const simplecpp::Token *tok)
{
return tok && (tok->number || isErrorValue(tok));
}

static bool isAlternativeBinaryOp(const simplecpp::Token *tok, const std::string &alt)
{
return (tok->name &&
tok->str() == alt &&
tok->previous &&
tok->next &&
(tok->previous->number || tok->previous->name || tok->previous->op == ')') &&
(tok->next->number || tok->next->name || tok->next->op == '('));
(isValue(tok->previous) || tok->previous->name || tok->previous->op == ')') &&
(isValue(tok->next) || tok->next->name || tok->next->op == '('));
}

static bool isAlternativeUnaryOp(const simplecpp::Token *tok, const std::string &alt)
Expand Down Expand Up @@ -1072,6 +1089,23 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
combineOperators();
}

/**
* If one operand of the binary operator tok is an error value then
* "lhs op rhs" is replaced by that error value.
* @return the error value token, or nullptr
*/
static simplecpp::Token *foldErrorValue(simplecpp::TokenList &tokens, simplecpp::Token *tok)
{
simplecpp::Token * const lhs = tok->previous;
if (!isErrorValue(lhs) && !isErrorValue(tok->next))
return nullptr;
if (!isErrorValue(lhs))
lhs->setstr(tok->next->str());
tokens.deleteToken(tok->next);
tokens.deleteToken(tok);
return lhs;
}

void simplecpp::TokenList::constFold()
{
while (cfront()) {
Expand Down Expand Up @@ -1105,6 +1139,11 @@ void simplecpp::TokenList::constFold()
deleteToken(tok->previous);
deleteToken(tok->next);
}

for (const Token *tok = cfront(); tok; tok = tok->next) {
if (isErrorValue(tok))
throw std::overflow_error(tok->str().substr(ERROR_VALUE.size()));
}
}

static bool isFloatSuffix(const simplecpp::Token *tok)
Expand Down Expand Up @@ -1262,17 +1301,27 @@ void simplecpp::TokenList::constFoldUnaryNotPosNeg(simplecpp::Token *tok)
else if (isAlternativeUnaryOp(tok, COMPL))
tok->op = '~';

if (tok->op == '!' && tok->next && tok->next->number) {
if (tok->isOneOf("!~") && isErrorValue(tok->next)) {
tok->setstr(tok->next->str());
deleteToken(tok->next);
} else if (tok->op == '!' && tok->next && tok->next->number) {
tok->setstr(tok->next->str() == "0" ? "1" : "0");
deleteToken(tok->next);
} else if (tok->op == '~' && tok->next && tok->next->number) {
tok->setstr(toString(~stringToLL(tok->next->str())));
deleteToken(tok->next);
} else {
if (tok->previous && (tok->previous->number || tok->previous->name))
if (tok->previous && (isValue(tok->previous) || tok->previous->name))
continue;
if (!tok->next || !tok->next->number)
if (!isValue(tok->next))
continue;
if (isErrorValue(tok->next)) {
if (tok->isOneOf("+-")) {
tok->setstr(tok->next->str());
deleteToken(tok->next);
}
continue;
}
switch (tok->op) {
case '+':
tok->setstr(tok->next->str());
Expand All @@ -1290,32 +1339,33 @@ void simplecpp::TokenList::constFoldUnaryNotPosNeg(simplecpp::Token *tok)
void simplecpp::TokenList::constFoldMulDivRem(Token *tok)
{
for (; tok && tok->op != ')'; tok = tok->next) {
if (!tok->previous || !tok->previous->number)
if (!isValue(tok->previous) || !isValue(tok->next))
continue;
if (!tok->isOneOf("*/%"))
continue;
if (!tok->next || !tok->next->number)
if (Token * const errorValue = foldErrorValue(*this, tok)) {
tok = errorValue;
continue;
}

long long result;
std::string result;
if (tok->op == '*') {
result = (stringToLL(tok->previous->str()) * stringToLL(tok->next->str()));
}
else if (tok->op == '/' || tok->op == '%') {
result = toString(stringToLL(tok->previous->str()) * stringToLL(tok->next->str()));
} else {
const long long rhs = stringToLL(tok->next->str());
if (rhs == 0)
throw std::overflow_error("division/modulo by zero");
const long long lhs = stringToLL(tok->previous->str());
if (rhs == -1 && lhs == std::numeric_limits<long long>::min())
throw std::overflow_error("division overflow");
if (tok->op == '/')
result = (lhs / rhs);
if (rhs == 0)
result = ERROR_VALUE + "division/modulo by zero";
else if (rhs == -1 && lhs == std::numeric_limits<long long>::min())
result = ERROR_VALUE + "division overflow";
else if (tok->op == '/')
result = toString(lhs / rhs);
else
result = (lhs % rhs);
} else {
continue;
result = toString(lhs % rhs);
}

tok = tok->previous;
tok->setstr(toString(result));
tok->setstr(result);
deleteToken(tok->next);
deleteToken(tok->next);
}
Expand All @@ -1324,10 +1374,14 @@ void simplecpp::TokenList::constFoldMulDivRem(Token *tok)
void simplecpp::TokenList::constFoldAddSub(Token *tok)
{
for (; tok && tok->op != ')'; tok = tok->next) {
if (!tok->previous || !tok->previous->number)
if (!isValue(tok->previous) || !isValue(tok->next))
continue;
if (!tok->isOneOf("+-"))
continue;
if (!tok->next || !tok->next->number)
if (Token * const errorValue = foldErrorValue(*this, tok)) {
tok = errorValue;
continue;
}

long long result;
if (tok->op == '+')
Expand All @@ -1347,10 +1401,14 @@ void simplecpp::TokenList::constFoldAddSub(Token *tok)
void simplecpp::TokenList::constFoldShift(Token *tok)
{
for (; tok && tok->op != ')'; tok = tok->next) {
if (!tok->previous || !tok->previous->number)
if (!isValue(tok->previous) || !isValue(tok->next))
continue;
if (tok->str() != "<<" && tok->str() != ">>")
continue;
if (!tok->next || !tok->next->number)
if (Token * const errorValue = foldErrorValue(*this, tok)) {
tok = errorValue;
continue;
}

long long result;
if (tok->str() == "<<")
Expand All @@ -1374,12 +1432,14 @@ void simplecpp::TokenList::constFoldComparison(Token *tok)
if (isAlternativeBinaryOp(tok,NOTEQ))
tok->setstr("!=");

if (!tok->startsWithOneOf("<>=!"))
if (!tok->isOneOf("<>") && tok->str() != "<=" && tok->str() != ">=" && tok->str() != "==" && tok->str() != "!=")
continue;
if (!tok->previous || !tok->previous->number)
if (!isValue(tok->previous) || !isValue(tok->next))
continue;
if (!tok->next || !tok->next->number)
if (Token * const errorValue = foldErrorValue(*this, tok)) {
tok = errorValue;
continue;
}

int result;
if (tok->str() == "==")
Expand Down Expand Up @@ -1419,10 +1479,12 @@ void simplecpp::TokenList::constFoldBitwise(Token *tok)
for (tok = tok1; tok && tok->op != ')'; tok = tok->next) {
if (tok->op != *op && !isAlternativeBinaryOp(tok, *alternativeOp))
continue;
if (!tok->previous || !tok->previous->number)
if (!isValue(tok->previous) || !isValue(tok->next))
continue;
if (!tok->next || !tok->next->number)
if (Token * const errorValue = foldErrorValue(*this, tok)) {
tok = errorValue;
continue;
}
long long result;
if (*op == '&')
result = (stringToLL(tok->previous->str()) & stringToLL(tok->next->str()));
Expand Down Expand Up @@ -1455,19 +1517,24 @@ void simplecpp::TokenList::constFoldLogicalOp(Token *tok)
for (Token *t = tok; t && t->op != ')'; t = t->next) {
if (t->str() != op)
continue;
if (!t->previous || !t->previous->number)
continue;
if (!t->next || !t->next->number)
if (!isValue(t->previous) || !isValue(t->next))
continue;

int result;
if (t->str() == "||")
result = (stringToLL(t->previous->str()) || stringToLL(t->next->str()));
else /*if (t->str() == "&&")*/
result = (stringToLL(t->previous->str()) && stringToLL(t->next->str()));
// the right operand is not evaluated if the left operand decides the result
std::string result;
if (isErrorValue(t->previous))
result = t->previous->str();
else if (t->str() == "||" && stringToLL(t->previous->str()) != 0)
result = "1";
else if (t->str() == "&&" && stringToLL(t->previous->str()) == 0)
result = "0";
else if (isErrorValue(t->next))
result = t->next->str();
else
result = (stringToLL(t->next->str()) != 0) ? "1" : "0";

t = t->previous;
t->setstr(toString(result));
t->setstr(result);
deleteToken(t->next);
deleteToken(t->next);
}
Expand All @@ -1476,30 +1543,42 @@ void simplecpp::TokenList::constFoldLogicalOp(Token *tok)

void simplecpp::TokenList::constFoldQuestionOp(Token *&tok1)
{
bool gotoTok1 = false;
// NOLINTNEXTLINE(misc-const-correctness) - technically correct but used to access non-const data
for (Token *tok = tok1; tok && tok->op != ')'; tok = gotoTok1 ? tok1 : tok->next) {
gotoTok1 = false;
if (tok->str() != "?")
continue;
// ?: is right-associative, "a ? b : c ? d : e" means "a ? b : (c ? d : e)",
// so the last ? is folded first
std::vector<const Token *> questionTokens;
for (const Token *t = tok1; t && t->op != ')'; t = t->next) {
if (t->str() == "?")
questionTokens.push_back(t);
}
while (!questionTokens.empty()) {
const Token * const tok = questionTokens.back();
questionTokens.pop_back();
if (!tok->previous || !tok->next || !tok->next->next)
throw std::runtime_error("invalid expression");
if (!tok->previous->number)
continue;
if (tok->next->next->op != ':')
continue;
Token * const condTok = tok->previous;
Token * const trueTok = tok->next;
Token * const falseTok = trueTok->next->next;
if (!falseTok)
throw std::runtime_error("invalid expression");
if (condTok == tok1)
tok1 = (condTok->str() != "0" ? trueTok : falseTok);
deleteToken(condTok->next); // ?
deleteToken(trueTok->next); // :
deleteToken(condTok->str() == "0" ? trueTok : falseTok);
deleteToken(condTok);
gotoTok1 = true;
if (trueTok->next->op != ':')
return;
if (!isValue(condTok) || !isValue(trueTok) || !isValue(falseTok))
return;
if (isErrorValue(condTok)) {
// "error ? x : y" => error
deleteToken(condTok->next); // ?
deleteToken(trueTok->next); // :
deleteToken(trueTok);
deleteToken(falseTok);
} else {
Token * const result = (condTok->str() != "0" ? trueTok : falseTok);
if (condTok == tok1)
tok1 = result;
deleteToken(condTok->next); // ?
deleteToken(trueTok->next); // :
deleteToken(result == falseTok ? trueTok : falseTok); // the operand that is not evaluated
deleteToken(condTok);
}
}
}

Expand Down
53 changes: 53 additions & 0 deletions test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,22 @@ static void constFold()
ASSERT_EQUALS("1", testConstFold("010==8"));
ASSERT_EQUALS("exception", testConstFold("!1 ? 2 :"));
ASSERT_EQUALS("exception", testConstFold("?2:3"));
ASSERT_EQUALS("0", testConstFold("0 && 1/0"));
ASSERT_EQUALS("1", testConstFold("1 || 1/0"));
ASSERT_EQUALS("2", testConstFold("1 ? 2 : 1/0"));
ASSERT_EQUALS("exception", testConstFold("1 && 1/0"));
ASSERT_EQUALS("exception", testConstFold("1/0 && 0"));
ASSERT_EQUALS("exception", testConstFold("(1/0) ? 1 : 2"));
ASSERT_EQUALS("exception", testConstFold("-(1/0) + 1"));
// ?: is right-associative
ASSERT_EQUALS("1", testConstFold("1 ? 1 : 0 ? 2 : 3"));
ASSERT_EQUALS("3", testConstFold("0 ? 1 : 0 ? 2 : 3"));
ASSERT_EQUALS("2", testConstFold("1 ? 2 : 3 ? 4 : 5 ? 6 : 7"));
ASSERT_EQUALS("7", testConstFold("0 ? 2 : 0 ? 4 : 0 ? 6 : 7"));
ASSERT_EQUALS("6", testConstFold("1 ? 0 ? 5 : 6 : 7"));
ASSERT_EQUALS("2", testConstFold("(1 ? 1 : 0) ? 2 : 3"));
ASSERT_EQUALS("1", testConstFold("1 ? 1 : (1/0) ? 2 : 3"));
ASSERT_EQUALS("exception", testConstFold("0 ? 1 : (1/0) ? 2 : 3"));
}

#ifdef __CYGWIN__
Expand Down Expand Up @@ -2835,6 +2851,42 @@ static void ifdiv0()
ASSERT_EQUALS("", preprocess(code));
}

static void ifShortCircuit()
{
// operands that are not evaluated must not cause errors
const char * const exprs[] = {
"0 && (1/0)",
"!(1 || (1/0))",
"0 ? 1/0 : 0",
"1 ? 0 : 1/0",
"0 && 1 % 0 && 1",
"(1 || 1/0) && !(0 && 1/0) && 0",
"0 && (2 + (3 * (1/0)))",
"0 && -(1/0)",
"0 and (1/0)",
"!(1 or 1/0)",
"defined(X) and X/0",
"0 && 1 bitor (1/0)",
"!(1 || (1/0) bitand 1)",
"0 and 1 xor (1/0)"
};
for (const char * const expr : exprs) {
const std::string code = std::string("#if ") + expr + "\n"
"1\n"
"#else\n"
"2\n"
"#endif\n";
simplecpp::OutputList outputList;
ASSERT_EQUALS("\n\n\n2", preprocess(code.c_str(), &outputList));
ASSERT_EQUALS("", toString(outputList));
}

// evaluated operands are still checked
simplecpp::OutputList outputList;
ASSERT_EQUALS("", preprocess("#if 1 && (1/0)\n#endif\n", &outputList));
ASSERT_EQUALS("file0,1,syntax_error,failed to evaluate #if condition, division/modulo by zero\n", toString(outputList));
}

static void ifTrueFalse()
{
const char code[] = "#if true && !false\n"
Expand Down Expand Up @@ -5070,6 +5122,7 @@ static void runTests(int argc, char **argv, Input input)
TEST_CASE(ifif);
TEST_CASE(ifoverflow);
TEST_CASE(ifdiv0);
TEST_CASE(ifShortCircuit);
TEST_CASE(ifTrueFalse);
TEST_CASE(ifalt); // using "and", "or", etc
TEST_CASE(ifexpr);
Expand Down
Loading