Skip to content

Fix #11438 MathLib error on user defined literals #5418

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

Closed
chrchr-github wants to merge 5 commits into from
Closed

Fix #11438 MathLib error on user defined literals #5418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
170e828
Fix #11438 MathLib error on user defined literals
chrchr-github Sep 8, 2023
f7f0250
Format
chrchr-github Sep 8, 2023
b2fd2d2
Don't handle non-standard literals
chrchr-github Sep 8, 2023
e1c2745
Adjust tests
chrchr-github Sep 8, 2023
329d29b
Merge branch 'chr_Fix11438' of https://github.com/chrchr-github/cppch…
chrchr-github Sep 13, 2023
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
38 changes: 29 additions & 9 deletions lib/mathlib.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ bool MathLib::isDecimalFloat(const std::string &str)
if (str.empty())
return false;
enum class State {
START, BASE_DIGITS1, LEADING_DECIMAL, TRAILING_DECIMAL, BASE_DIGITS2, E, MANTISSA_PLUSMINUS, MANTISSA_DIGITS, SUFFIX_F, SUFFIX_L, SUFFIX_LITERAL_LEADER, SUFFIX_LITERAL
START, BASE_DIGITS1, LEADING_DECIMAL, TRAILING_DECIMAL, BASE_DIGITS2, E, MANTISSA_PLUSMINUS, MANTISSA_DIGITS, SUFFIX_D, SUFFIX_F, SUFFIX_L, SUFFIX_LITERAL_LEADER, SUFFIX_LITERAL
} state = State::START;
std::string::const_iterator it = str.cbegin();
if ('+' == *it || '-' == *it)
Expand Down Expand Up @@ -576,6 +576,8 @@ bool MathLib::isDecimalFloat(const std::string &str)
state = State::E;
else if (*it=='f' || *it=='F')
state = State::SUFFIX_F;
else if (*it=='d' || *it=='D')
state = State::SUFFIX_D;
else if (*it=='l' || *it=='L')
state = State::SUFFIX_L;
else if (*it == '_')
Expand All @@ -590,6 +592,8 @@ bool MathLib::isDecimalFloat(const std::string &str)
state = State::E;
else if (*it=='f' || *it=='F')
state = State::SUFFIX_F;
else if (*it=='d' || *it=='D')
state = State::SUFFIX_D;
else if (*it=='l' || *it=='L')
state = State::SUFFIX_L;
else if (*it == '_')
Expand Down Expand Up @@ -625,12 +629,13 @@ bool MathLib::isDecimalFloat(const std::string &str)
state = State::SUFFIX_LITERAL;
break;
case State::SUFFIX_F:
return false;
case State::SUFFIX_D:
case State::SUFFIX_L:
return false;
}
}
return (state==State::BASE_DIGITS2 || state==State::MANTISSA_DIGITS || state==State::TRAILING_DECIMAL || state==State::SUFFIX_F || state==State::SUFFIX_L || (state==State::SUFFIX_LITERAL));
return state == State::BASE_DIGITS2 || state == State::MANTISSA_DIGITS || state == State::TRAILING_DECIMAL ||
state == State::SUFFIX_F || state == State::SUFFIX_D || state == State::SUFFIX_L || state == State::SUFFIX_LITERAL;
}

bool MathLib::isNegative(const std::string &str)
Expand All @@ -649,11 +654,13 @@ bool MathLib::isPositive(const std::string &str)

static bool isValidIntegerSuffixIt(std::string::const_iterator it, std::string::const_iterator end, bool supportMicrosoftExtensions=true)
{
enum class Status { START, SUFFIX_U, SUFFIX_UL, SUFFIX_ULL, SUFFIX_UZ, SUFFIX_L, SUFFIX_LU, SUFFIX_LL, SUFFIX_LLU, SUFFIX_I, SUFFIX_I6, SUFFIX_I64, SUFFIX_UI, SUFFIX_UI6, SUFFIX_UI64, SUFFIX_Z, SUFFIX_LITERAL_LEADER, SUFFIX_LITERAL } state = Status::START;
enum class Status { START, SUFFIX_B, SUFFIX_U, SUFFIX_UL, SUFFIX_ULL, SUFFIX_UZ, SUFFIX_L, SUFFIX_LU, SUFFIX_LL, SUFFIX_LLU, SUFFIX_I, SUFFIX_I6, SUFFIX_I64, SUFFIX_UI, SUFFIX_UI6, SUFFIX_UI64, SUFFIX_Z, SUFFIX_LITERAL_LEADER, SUFFIX_LITERAL } state = Status::START;
for (; it != end; ++it) {
switch (state) {
case Status::START:
if (*it == 'u' || *it == 'U')
if (*it == 'b' || *it == 'B')
state = Status::SUFFIX_B;
else if (*it == 'u' || *it == 'U')
state = Status::SUFFIX_U;
else if (*it == 'l' || *it == 'L')
state = Status::SUFFIX_L;
Expand Down Expand Up @@ -737,7 +744,8 @@ static bool isValidIntegerSuffixIt(std::string::const_iterator it, std::string::
return false;
}
}
return ((state == Status::SUFFIX_U) ||
return ((state == Status::SUFFIX_B) ||
(state == Status::SUFFIX_U) ||
(state == Status::SUFFIX_L) ||
(state == Status::SUFFIX_Z) ||
(state == Status::SUFFIX_UL) ||
Expand Down Expand Up @@ -937,7 +945,7 @@ bool MathLib::isFloatHex(const std::string& str)
bool MathLib::isBin(const std::string& str)
{
enum class Status {
START, GNU_BIN_PREFIX_0, GNU_BIN_PREFIX_B, DIGIT
START, GNU_BIN_PREFIX_0, GNU_BIN_PREFIX_B, DIGIT, DIGIT_TENTATIVE, SUFFIX_B
} state = Status::START;
if (str.empty())
return false;
Expand All @@ -949,6 +957,8 @@ bool MathLib::isBin(const std::string& str)
case Status::START:
if (*it == '0')
state = Status::GNU_BIN_PREFIX_0;
else if (*it == '1')
state = Status::DIGIT_TENTATIVE;
else
return false;
break;
Expand All @@ -966,13 +976,23 @@ bool MathLib::isBin(const std::string& str)
break;
case Status::DIGIT:
if (*it == '0' || *it == '1')
; // state = Status::DIGIT;
; // state = Status::DIGIT;
else
return isValidIntegerSuffixIt(it,str.end());
break;
case Status::DIGIT_TENTATIVE:
if (*it == '0' || *it == '1')
; // state = Status::DIGIT_TENTATIVE;
else if (*it == 'b' || *it == 'B')
state = Status::SUFFIX_B;
else
return false;
break;
case Status::SUFFIX_B:
return false;
}
}
return state == Status::DIGIT;
return state == Status::DIGIT || state == Status::SUFFIX_B;
}

bool MathLib::isDec(const std::string & str)
Expand Down
14 changes: 10 additions & 4 deletions lib/token.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,16 @@ void Token::update_property_info()
tokType(eKeyword);
else if (mTokType != eVariable && mTokType != eFunction && mTokType != eType && mTokType != eKeyword)
tokType(eName);
} else if (std::isdigit((unsigned char)mStr[0]) || (mStr.length() > 1 && mStr[0] == '-' && std::isdigit((unsigned char)mStr[1])))
tokType(eNumber);
else if (mStr == "=" || mStr == "<<=" || mStr == ">>=" ||
(mStr.size() == 2U && mStr[1] == '=' && std::strchr("+-*/%&^|", mStr[0])))
} else if (std::isdigit((unsigned char)mStr[0]) || (mStr.length() > 1 && mStr[0] == '-' && std::isdigit((unsigned char)mStr[1]))) {
Copy link
Collaborator

@firewave firewave Sep 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic might also exists in simplecpp. We need to keep that in sync or provide in function to get rid of the redundant code. I might be mistaken though.

Copy link
Collaborator Author

@chrchr-github chrchr-github Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see any code that is directly related.

Copy link
Collaborator

@firewave firewave Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplecpp::Token::flags() in simplecpp.h.

There was talk about sharing this logic with some other dev but I guess that hasn't happened yet. Oops, the PR is actually still open: #4701. Also danmar/simplecpp#285.

Copy link
Collaborator Author

@chrchr-github chrchr-github Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. 101010b was also discussed in there. This is getting ugly.

Copy link
Collaborator

@firewave firewave Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, that string-to-number stuff also plays into this. At least I got rid of the bogus calls and added a different conversion function but I haven't removed the code from the MathLib conversion functions which does not apply to number literals yet.

Copy link
Collaborator

@firewave firewave Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. It seems I didn't add anything which is not about number literals (my memory is a mess). If I did it must have been in #4611. But I don't think it make sense to review that.

We should simply get rid of code which makes invalid test cases pass.

Copy link
Collaborator

@firewave firewave Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you re-use any code from that other PR please credit Gerbo via Co-authored-by: at the end of the commit message.

try {
if (!MathLib::isFloat(mStr))
(void)MathLib::toLongNumber(mStr);
tokType(eNumber);
} catch (const InternalError& /*e*/) {
tokType(eName);
}
} else if (mStr == "=" || mStr == "<<=" || mStr == ">>=" ||
(mStr.size() == 2U && mStr[1] == '=' && std::strchr("+-*/%&^|", mStr[0])))
tokType(eAssignmentOp);
else if (mStr.size() == 1 && mStr.find_first_of(",[]()?:") != std::string::npos)
tokType(eExtendedOp);
Expand Down
10 changes: 8 additions & 2 deletions test/testtoken.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,14 @@ class TestToken : public TestFixture {
givenACodeSampleToTokenize nonNumeric("abc", true);
ASSERT_EQUALS(false, Token::Match(nonNumeric.tokens(), "%num%"));

givenACodeSampleToTokenize msLiteral("5ms", true); // #11438
ASSERT_EQUALS(false, Token::Match(msLiteral.tokens(), "%num%"));

givenACodeSampleToTokenize sLiteral("3s", true);
ASSERT_EQUALS(false, Token::Match(sLiteral.tokens(), "%num%"));

givenACodeSampleToTokenize binary("101010b", true);
ASSERT_EQUALS(true, Token::Match(binary.tokens(), "%num%"));
ASSERT_EQUALS(false, Token::Match(binary.tokens(), "%num%"));

givenACodeSampleToTokenize octal("0123", true);
ASSERT_EQUALS(true, Token::Match(octal.tokens(), "%num%"));
Expand All @@ -653,7 +659,7 @@ class TestToken : public TestFixture {
ASSERT_EQUALS(true, Token::Match(floatingPoint.tokens(), "%num%"));

givenACodeSampleToTokenize doublePrecision("0.0d", true);
ASSERT_EQUALS(true, Token::Match(doublePrecision.tokens(), "%num%"));
ASSERT_EQUALS(false, Token::Match(doublePrecision.tokens(), "%num%"));

givenACodeSampleToTokenize signedLong("0L", true);
ASSERT_EQUALS(true, Token::Match(signedLong.tokens(), "%num%"));
Expand Down