Skip to content

Commit

Permalink
[CORRECTIVE] Fix unary minus parsing in expressions.
Browse files Browse the repository at this point in the history
SystemVerilogExpressionParser takes a copy of the original expression and convertToRPN replaces unary minuses with new character ` (backtick), which solveRPN treats as unary minus. Might come with a slight performance hit on the parser, need to profile.
  • Loading branch information
hagantsa committed Jul 15, 2024
1 parent 84f654d commit 835fd54
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 22 deletions.
41 changes: 35 additions & 6 deletions KactusAPI/expressions/SystemVerilogExpressionParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace
const QRegularExpression BINARY_OPERATOR(QStringLiteral("[/%^+-]|<<|>>|<=?|>=?|!==?|===?|[&|*]{1,2}|[$]pow"));

const QRegularExpression UNARY_OPERATOR(QStringLiteral(
"[$]clog2|[$]exp|[$]sqrt|[$]ipxact_mode_condition|[$]ipxact_port_value|[$]ipxact_field_value|~"));
"[$]clog2|[$]exp|[$]sqrt|[$]ipxact_mode_condition|[$]ipxact_port_value|[$]ipxact_field_value|~|`"));

const QRegularExpression TERNARY_OPERATOR(QStringLiteral("[?:]"));

Expand Down Expand Up @@ -64,7 +64,9 @@ namespace
//-----------------------------------------------------------------------------
QString SystemVerilogExpressionParser::parseExpression(QStringView expression, bool* validExpression) const
{
return solveRPN(convertToRPN(expression), validExpression);
// Copy of expression needs to be created for replacing unary minuses with special character.
QString expressionCopy = expression.toString();
return solveRPN(convertToRPN(expressionCopy), validExpression);
}

//-----------------------------------------------------------------------------
Expand All @@ -89,8 +91,8 @@ bool SystemVerilogExpressionParser::isPlainValue(QStringView expression) const
int SystemVerilogExpressionParser::baseForExpression(QStringView expression) const
{
int greatestBase = 0;

for (auto const& token : convertToRPN(expression))
QString asStr = expression.toString();
for (auto const& token : convertToRPN(asStr))
{
if (isLiteral(token))
{
Expand All @@ -108,7 +110,7 @@ int SystemVerilogExpressionParser::baseForExpression(QStringView expression) con
//-----------------------------------------------------------------------------
// Function: SystemVerilogExpressionParser::convertToRPN()
//-----------------------------------------------------------------------------
QVector<QStringView> SystemVerilogExpressionParser::convertToRPN(QStringView expression)
QVector<QStringView> SystemVerilogExpressionParser::convertToRPN(QString& expression)
{
// Convert expression to Reverse Polish Notation (RPN) using the Shunting Yard algorithm.
QVector<QStringView> output;
Expand All @@ -117,7 +119,28 @@ QVector<QStringView> SystemVerilogExpressionParser::convertToRPN(QStringView exp
int openParenthesis = 0;
bool nextMayBeLiteral = true;

qsizetype previousIndex = -1;
const auto SIZE = expression.size();

// Convert unary minuses to backticks. Needs to be done before main loop, otherwise string views will be invalidated.
// SolveRPN knows to treat backticks as unary minus.
for (qsizetype index = 0; index < SIZE; ++index)
{
const QChar current = expression.at(index);
if (current.isSpace())
{
continue;
}

// Minus == unary minus if preceded by opening parenthesis or if first token
if (current == QLatin1Char('-') && (previousIndex == -1 || expression.at(previousIndex) == OPEN_PARENTHESIS_STRING))
{
expression[index] = QLatin1Char('`');
}

previousIndex = index;
}

for (qsizetype index = 0; index < SIZE; /*index incremented inside loop*/)
{
const QChar current = expression.at(index);
Expand Down Expand Up @@ -215,7 +238,8 @@ QVector<QStringView> SystemVerilogExpressionParser::convertToRPN(QStringView exp
else
{
static const QRegularExpression separator(ANY_OPERATOR.pattern() % QStringLiteral("|[(){},]"));
const auto unknown = expression.mid(index, separator.match(expression, index).capturedStart() - index);
auto unknown = QStringView(expression);
unknown = unknown.mid(index, separator.match(expression, index).capturedStart() - index);

output.append(unknown.trimmed());

Expand Down Expand Up @@ -589,6 +613,10 @@ QString SystemVerilogExpressionParser::solveUnary(QStringView operation, QString
{
return QString::number(~term.toLongLong());
}
else if (operation.compare(QLatin1String("`")) == 0)
{
return QString::number(~term.toLongLong() + 1);
}

return QStringLiteral("x");
}
Expand Down Expand Up @@ -710,6 +738,7 @@ unsigned int SystemVerilogExpressionParser::operatorPrecedence(QStringView oper)
{QStringLiteral("*") , 12},
{QStringLiteral("/") , 12},
{QStringLiteral("**") , 13},
{QStringLiteral("`") , 13}, // unary minus
{QStringLiteral("~") , 14},
{QStringLiteral("$clog2") , 14},
{QStringLiteral("$pow") , 14},
Expand Down
4 changes: 2 additions & 2 deletions KactusAPI/include/ParameterizableInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class KACTUS2_API ParameterizableInterface
*
* @return The value of the expression in decimal form.
*/
QString parseExpressionToDecimal(QString const& expression) const;
QString parseExpressionToDecimal(QString const& expression, bool* expressionIsValid = nullptr) const;

/*!
* Parse the selected expression to the selected base number.
Expand All @@ -83,7 +83,7 @@ class KACTUS2_API ParameterizableInterface
*
* @return The value of the expression in the base number form.
*/
QString parseExpressionToBaseNumber(QString const& expression, unsigned int baseNumber) const;
QString parseExpressionToBaseNumber(QString const& expression, unsigned int baseNumber, bool* expressionIsValid = nullptr) const;

private:

Expand Down
2 changes: 1 addition & 1 deletion KactusAPI/include/SystemVerilogExpressionParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class KACTUS2_API SystemVerilogExpressionParser : public ExpressionParser
*
* @return The conversion result.
*/
static QVector<QStringView> convertToRPN(QStringView expression);
static QVector<QStringView> convertToRPN(QString& expression);

/*!
* Solves the given RPN expression.
Expand Down
5 changes: 4 additions & 1 deletion KactusAPI/interfaces/common/AbstractParameterInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,10 @@ std::string AbstractParameterInterface::getValue(std::string const& parameterNam
}
else
{
return parseExpressionToBaseNumber(parameter->getValue(), baseNumber).toStdString();
bool expressionIsValid = false;
auto value = parseExpressionToBaseNumber(parameter->getValue(), baseNumber, &expressionIsValid).toStdString();

return expressionIsValid ? value : std::string("x");
}
}

Expand Down
8 changes: 4 additions & 4 deletions KactusAPI/interfaces/common/ParameterizableInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ QString ParameterizableInterface::formattedValueFor(QString const& expression) c
//-----------------------------------------------------------------------------
// Function: ParameterizableInterface::parseExpressionToDecimal()
//-----------------------------------------------------------------------------
QString ParameterizableInterface::parseExpressionToDecimal(QString const& expression) const
QString ParameterizableInterface::parseExpressionToDecimal(QString const& expression, bool* expressionIsValid /*= nullptr*/) const
{
return expressionParser_->parseExpression(expression);
return expressionParser_->parseExpression(expression, expressionIsValid);
}

//-----------------------------------------------------------------------------
// Function: ParameterizableInterface::parseExpressionToBaseNumber()
//-----------------------------------------------------------------------------
QString ParameterizableInterface::parseExpressionToBaseNumber(QString const& expression, unsigned int baseNumber) const
QString ParameterizableInterface::parseExpressionToBaseNumber(QString const& expression, unsigned int baseNumber, bool* expressionIsValid /*= nullptr*/) const
{
return valueFormatter_->format(parseExpressionToDecimal(expression), baseNumber);
return valueFormatter_->format(parseExpressionToDecimal(expression, expressionIsValid), baseNumber);
}
16 changes: 8 additions & 8 deletions version.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,20 @@
#ifndef VERSIONNO__H
#define VERSIONNO__H

#define VERSION_FULL 3.13.517.0
#define VERSION_FULL 3.13.519.0

#define VERSION_BASEYEAR 0
#define VERSION_DATE "2024-07-12"
#define VERSION_TIME "09:50:38"
#define VERSION_DATE "2024-07-15"
#define VERSION_TIME "17:24:58"

#define VERSION_MAJOR 3
#define VERSION_MINOR 13
#define VERSION_BUILDNO 517
#define VERSION_BUILDNO 519
#define VERSION_EXTEND 0

#define VERSION_FILE 3,13,517,0
#define VERSION_PRODUCT 3,13,517,0
#define VERSION_FILESTR "3,13,517,0"
#define VERSION_PRODUCTSTR "3,13,517,0"
#define VERSION_FILE 3,13,519,0
#define VERSION_PRODUCT 3,13,519,0
#define VERSION_FILESTR "3,13,519,0"
#define VERSION_PRODUCTSTR "3,13,519,0"

#endif

0 comments on commit 835fd54

Please sign in to comment.