Skip to content

Fix handling of expression types #7

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

Open
wants to merge 3 commits into
base: tarcv-master
Choose a base branch
from
Open

Fix handling of expression types #7

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1060356
Fix expression type handling
TarCV Apr 21, 2019
721b64a
(Partially) fix reporting position of an error
TarCV Apr 21, 2019
9a6f8a4
Merge branch 'tarcv-master' of github.com:TarCV/botc into fixes3
TarCV Apr 24, 2020
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
120 changes: 75 additions & 45 deletions src/expression.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,33 @@ struct OperatorInfo
Token token;
int priority;
int numoperands;
DataType returnType;
DataHeader header;
};

static const OperatorInfo g_Operators[] =
{
{Token::ExclamationMark, 0, 1, DataHeader::NegateLogical, },
{Token::Minus, 0, 1, DataHeader::UnaryMinus, },
{Token::Multiply, 10, 2, DataHeader::Multiply, },
{Token::Divide, 10, 2, DataHeader::Divide, },
{Token::Modulus, 10, 2, DataHeader::Modulus, },
{Token::Plus, 20, 2, DataHeader::Add, },
{Token::Minus, 20, 2, DataHeader::Subtract, },
{Token::LeftShift, 30, 2, DataHeader::LeftShift, },
{Token::RightShift, 30, 2, DataHeader::RightShift, },
{Token::Lesser, 40, 2, DataHeader::LessThan, },
{Token::Greater, 40, 2, DataHeader::GreaterThan, },
{Token::AtLeast, 40, 2, DataHeader::AtLeast, },
{Token::AtMost, 40, 2, DataHeader::AtMost, },
{Token::Equals, 50, 2, DataHeader::Equals },
{Token::NotEquals, 50, 2, DataHeader::NotEquals },
{Token::Amperstand, 60, 2, DataHeader::AndBitwise },
{Token::Caret, 70, 2, DataHeader::EorBitwise },
{Token::Bar, 80, 2, DataHeader::OrBitwise },
{Token::DoubleAmperstand, 90, 2, DataHeader::AndLogical },
{Token::DoubleBar, 100, 2, DataHeader::OrLogical },
{Token::QuestionMark, 110, 3, DataHeader::NumValues },
{Token::ExclamationMark, 0, 1, TYPE_Bool, DataHeader::NegateLogical, },
{Token::Minus, 0, 1, TYPE_Int, DataHeader::UnaryMinus, },
{Token::Multiply, 10, 2, TYPE_Int, DataHeader::Multiply, },
{Token::Divide, 10, 2, TYPE_Int, DataHeader::Divide, },
{Token::Modulus, 10, 2, TYPE_Int, DataHeader::Modulus, },
{Token::Plus, 20, 2, TYPE_Int, DataHeader::Add, },
{Token::Minus, 20, 2, TYPE_Int, DataHeader::Subtract, },
{Token::LeftShift, 30, 2, TYPE_Int, DataHeader::LeftShift, },
{Token::RightShift, 30, 2, TYPE_Int, DataHeader::RightShift, },
{Token::Lesser, 40, 2, TYPE_Bool, DataHeader::LessThan, },
{Token::Greater, 40, 2, TYPE_Bool, DataHeader::GreaterThan, },
{Token::AtLeast, 40, 2, TYPE_Bool, DataHeader::AtLeast, },
{Token::AtMost, 40, 2, TYPE_Bool, DataHeader::AtMost, },
{Token::Equals, 50, 2, TYPE_Bool, DataHeader::Equals },
{Token::NotEquals, 50, 2, TYPE_Bool, DataHeader::NotEquals },
{Token::Amperstand, 60, 2, TYPE_Int, DataHeader::AndBitwise },
{Token::Caret, 70, 2, TYPE_Int, DataHeader::EorBitwise },
{Token::Bar, 80, 2, TYPE_Int, DataHeader::OrBitwise },
{Token::DoubleAmperstand, 90, 2, TYPE_Bool, DataHeader::AndLogical },
{Token::DoubleBar, 100, 2, TYPE_Bool, DataHeader::OrLogical },
{Token::QuestionMark, 110, 3, TYPE_ToBeDecided, DataHeader::NumValues },
};

// _________________________________________________________________________________________________
Expand Down Expand Up @@ -75,7 +76,6 @@ Expression::~Expression()
ExpressionSymbol* Expression::parseSymbol()
{
int pos = m_lexer->position();
ExpressionValue* op = null;

if (m_lexer->next (Token::Colon))
return new ExpressionColon;
Expand All @@ -84,28 +84,29 @@ ExpressionSymbol* Expression::parseSymbol()
for (const OperatorInfo& op : g_Operators)
{
if (m_lexer->next (op.token))
return new ExpressionOperator ((ExpressionOperatorType) (&op - &g_Operators[0]));
return new ExpressionOperator ((ExpressionOperatorType) (&op - &g_Operators[0]), op.returnType);
}

// Check sub-expression
if (m_lexer->next (Token::ParenStart))
{
Expression expr (m_parser, m_lexer, m_type);
Expression expr (m_parser, m_lexer, TYPE_ToBeDecided);
m_lexer->mustGetNext (Token::ParenEnd);
return expr.getResult()->clone();
}

op = new ExpressionValue (m_type);

// Check function
if (CommandInfo* comm = findCommandByName (m_lexer->peekNextString()))
{
m_lexer->skip();

if (m_type != TYPE_Unknown and comm->returnvalue != m_type)
error ("%1 returns an incompatible data type", comm->name);
if (m_type != TYPE_Unknown and m_type != TYPE_ToBeDecided and comm->returnvalue != m_type) {
error("%1 returns an incompatible data type", comm->name);
}

ExpressionValue* op = new ExpressionValue(comm->returnvalue);
op->setBuffer (m_parser->parseCommand (comm));
op->setValueType(comm->returnvalue);
return op;
}

Expand All @@ -118,12 +119,14 @@ ExpressionSymbol* Expression::parseSymbol()
if (var == null)
error ("unknown variable %1", getTokenString());

if (var->type != m_type)
if (m_type != TYPE_ToBeDecided && var->type != m_type)
{
error ("expression requires %1, variable $%2 is of type %3",
dataTypeName (m_type), var->name, dataTypeName (var->type));
}

ExpressionValue* op = new ExpressionValue(var->type);

if (var->isarray)
{
m_lexer->mustGetNext (Token::BracketStart);
Expand Down Expand Up @@ -155,50 +158,69 @@ ExpressionSymbol* Expression::parseSymbol()
return op;
}

// Check for literal
switch (m_type)
{
if (m_type == TYPE_ToBeDecided) {
if (m_lexer->peekNextType(Token::True) || m_lexer->peekNextType(Token::False)) {
m_type = TYPE_Bool;
}
else if (m_lexer->peekNextType(Token::Number)) {
m_type = TYPE_Int;
}
else if (m_lexer->peekNextType(Token::String)) {
m_type = TYPE_String;
}
else {
m_type = TYPE_ToBeDecided; // skip the following switch
}
}

ExpressionValue* op = new ExpressionValue(m_type);

// Check for literal
switch (m_type)
{
case TYPE_Void:
case TYPE_Unknown:
{
error ("unknown identifier `%1` (expected keyword, function or variable)",
error("unknown identifier `%1` (expected keyword, function or variable)",
getTokenString());
break;
}

case TYPE_Bool:
{
if (m_lexer->next (Token::True) or m_lexer->next (Token::False))
if (m_lexer->next(Token::True) or m_lexer->next(Token::False))
{
Token tt = m_lexer->tokenType();
op->setValue (tt == Token::True ? 1 : 0);
op->setValue(tt == Token::True ? 1 : 0);
return op;
}
}

case TYPE_Int:
{
if (m_lexer->next (Token::Number))
if (m_lexer->next(Token::Number))
{
op->setValue (getTokenString().toLong());
op->setValue(getTokenString().toLong());
return op;
}
}

case TYPE_String:
{
if (m_lexer->next (Token::String))
if (m_lexer->next(Token::String))
{
op->setValue (getStringTableIndex (getTokenString()));
op->setValue(getStringTableIndex(getTokenString()));
return op;
}
}
}
}

m_badTokenText = m_lexer->token()->text;
m_lexer->setPosition (pos);
delete op;
return null;
m_badTokenText = m_lexer->token()->text;
m_lexer->setPosition(pos);
delete op;
return null;
}
}

// _________________________________________________________________________________________________
Expand Down Expand Up @@ -422,6 +444,14 @@ ExpressionValue* Expression::evaluateOperator (const ExpressionOperator* op,

ExpressionValue* newval = new ExpressionValue (m_type);

DataType returnType = TYPE_ToBeDecided;
if (op->id() == OPER_Ternary) {
returnType = values[1]->valueType();
} else {
returnType = op->returnType();
}
newval->setValueType(returnType);

if (isconstexpr == false)
{
// This is not a constant expression so we'll have to use databuffers
Expand Down Expand Up @@ -612,9 +642,9 @@ String Expression::getTokenString()

// _________________________________________________________________________________________________
//
ExpressionOperator::ExpressionOperator (ExpressionOperatorType id) :
ExpressionOperator::ExpressionOperator (ExpressionOperatorType id, DataType returnType) :
ExpressionSymbol (EXPRSYM_Operator),
m_id (id) {}
m_id (id), m_returnType(returnType) {}

// _________________________________________________________________________________________________
//
Expand Down
5 changes: 2 additions & 3 deletions src/expression.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ class Expression final
class ExpressionSymbol
{
PROPERTY (private, ExpressionSymbolType, type, setType, STOCK_WRITE)

public:
ExpressionSymbol (ExpressionSymbolType type) :
m_type (type) {}
Expand All @@ -87,9 +86,9 @@ class ExpressionSymbol
class ExpressionOperator final : public ExpressionSymbol
{
PROPERTY (public, ExpressionOperatorType, id, setID, STOCK_WRITE)

PROPERTY(public, DataType, returnType, setReturnType, STOCK_WRITE)
public:
ExpressionOperator (ExpressionOperatorType id);
ExpressionOperator (ExpressionOperatorType id, DataType returnType);
};

// =============================================================================
Expand Down
2 changes: 1 addition & 1 deletion src/format.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void error (const String& msg)
if (lx != null and lx->hasValidToken())
{
Lexer::TokenInfo* tk = lx->token();
fileinfo = format ("%1:%2:%3: ", tk->file, tk->line, tk->column);
fileinfo = format ("%1:%2:near %3: ", tk->file, tk->line, tk->column);
}

throw std::runtime_error ((fileinfo + msg).c_str());
Expand Down
2 changes: 1 addition & 1 deletion src/lexer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class Lexer

inline bool hasValidToken() const
{
return (m_tokenPosition < m_tokens.end() and m_tokenPosition >= m_tokens.begin());
return (m_tokenPosition < m_tokens.end() and m_tokenPosition > m_tokens.begin());
}

inline TokenInfo* token() const
Expand Down
7 changes: 6 additions & 1 deletion src/lexerScanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,15 @@ bool LexerScanner::getNextToken()
//
void LexerScanner::skip()
{
if (*m_position == '\n' || (*m_position == '\r' && *(m_position+1) == '\n'))
if ((*m_position == '\r' && *(m_position+1) == '\n') || (*m_position == '\n' && *(m_position + 1) == '\r'))
{
m_line++;
m_lineBreakPosition = m_position;
m_position++; // for two char line ending have to skip one more character
}
else if (*m_position == '\n' || *m_position == '\r') {
m_line++;
m_lineBreakPosition = m_position;
}

m_position++;
Expand Down
10 changes: 7 additions & 3 deletions src/parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ void BotscriptParser::parseVar()
var->origin = m_lexer->describeCurrentPosition();
var->isarray = false;
bool isconst = m_lexer->next (Token::Const);
m_lexer->mustGetAnyOf ({Token::Int,Token::Str,Token::Void});
m_lexer->mustGetAnyOf ({Token::Int,Token::Str,Token::Bool});

DataType vartype = (tokenIs (Token::Int)) ? TYPE_Int
: (tokenIs (Token::Str)) ? TYPE_String
Expand Down Expand Up @@ -432,7 +432,7 @@ void BotscriptParser::parseIf()
m_lexer->mustGetNext (Token::ParenStart);

// Read the expression and write it.
DataBuffer* c = parseExpression (TYPE_Int);
DataBuffer* c = parseExpression (TYPE_Bool);
currentBuffer()->mergeAndDestroy (c);

m_lexer->mustGetNext (Token::ParenEnd);
Expand Down Expand Up @@ -1266,7 +1266,11 @@ DataBuffer* BotscriptParser::parseExpression (DataType reqtype, bool fromhere)
if (fromhere)
m_lexer->skip (-1);

Expression expr (this, m_lexer, reqtype);
Expression expr (this, m_lexer, TYPE_ToBeDecided);
ASSERT_NE(expr.getResult()->valueType(), TYPE_ToBeDecided)
if (expr.getResult()->valueType() != reqtype) {
error("Incompatible data type");
Copy link

Choose a reason for hiding this comment

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

error: crashbot.botc:64:near 51: Incompatible data type
if (ChatSectionExistsInChatLump("WinStrings")) {

Copy link
Owner Author

Choose a reason for hiding this comment

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

Try botc_defs.bts from this branch. 1060356 requires if "argument" to be boolean. (This is what Java does and I think such behavior is more friendly to Doom modders)
When an actual int returning function is used as if argument, you need to rewrite it like if (func() != 0).

Copy link

Choose a reason for hiding this comment

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

Something I noticed, although I don't know if you would consider it a bug. The compiler complains if the logical operator is used in an int (let's suppose it is named $local1) like $local1 = !($local1). Still converting the source code for the new defs, so far no real issues.

Copy link

Choose a reason for hiding this comment

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

It compiled and works with the Roam() fix, here is the script I've used http://sprunge.us/v4r7jN.

}
expr.getResult()->convertToBuffer();

// The buffer will be destroyed once the function ends so we need to
Expand Down
1 change: 1 addition & 0 deletions src/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ named_enum DataType : char
TYPE_Int,
TYPE_String,
TYPE_Bool,
TYPE_ToBeDecided // unknown by the time parsing started, but should be known by the end of parsing expression
};

// _________________________________________________________________________________________________
Expand Down