Allow returning void expressions where state changes are allowed

In the same places as state changes are allowed, i.e. in places where a parent of the AST node is a WHILE/DO/FOR or an IF without ELSE, it's allowed to use return statements with expressions that return void, e.g. llDie(), provided the function itself is declared as returning void.

The construction, when found, is rewritten to '{<void expression>; return;}' because the optimizer is not designed to deal with these monsters.

We've renamed the variable SuspiciousStSw to PruneBug, because it's used for both purposes now, though a better name might have been PruneBugPendingChecks, because these are only errors if the IF has an ELSE. We've also added the exception to raise as part of the data stored in the list.

Per report by Tonaie Resident.
This commit is contained in:
Sei Lisa 2019-05-01 04:03:58 +02:00
parent 3cfbbb923c
commit 9d540798b4
5 changed files with 183 additions and 14 deletions

View file

@ -1791,7 +1791,8 @@ list lazy_list_set(list L, integer i, list v)
if AllowStSw is False: if AllowStSw is False:
raise EParseCantChangeState(self) raise EParseCantChangeState(self)
if AllowStSw is None: if AllowStSw is None:
self.SuspiciousStSw.append(self.errorpos) self.PruneBug.append((self.errorpos,
EParseCantChangeState))
self.NextToken() self.NextToken()
if self.tok[0] not in ('DEFAULT', 'IDENT'): if self.tok[0] not in ('DEFAULT', 'IDENT'):
raise EParseSyntax(self) raise EParseSyntax(self)
@ -1812,12 +1813,31 @@ list lazy_list_set(list L, integer i, list v)
value = None value = None
else: else:
savepos = self.errorpos savepos = self.errorpos
saveAllowVoid = self.allowVoid
# Needed due to another LSL bug, see regr/void-in-return.lsl
self.allowVoid = True
value = self.Parse_expression() value = self.Parse_expression()
self.allowVoid = saveAllowVoid
self.expect(';') self.expect(';')
self.NextToken() self.NextToken()
if ReturnType is None and value is not None: if ReturnType is None and value is not None:
self.errorpos = savepos # It follows the same rules as AllowStSw
raise EParseReturnShouldBeEmpty(self) if AllowStSw is False:
self.errorpos = savepos
raise EParseReturnShouldBeEmpty(self)
elif value.t is None:
if AllowStSw is None:
self.PruneBug.append((self.errorpos,
EParseReturnShouldBeEmpty))
self.PushScope()
scope = self.scopeindex
self.PopScope()
return nr(nt='{}', t=None, scope=scope,
ch=[nr(nt='EXPR', t=None, ch=[value]),
nr(nt='RETURN', t=None)])
else:
self.errorpos = savepos
raise EParseTypeMismatch(self)
if ReturnType is not None and value is None: if ReturnType is not None and value is None:
self.errorpos = savepos self.errorpos = savepos
raise EParseReturnIsEmpty(self) raise EParseReturnIsEmpty(self)
@ -1835,22 +1855,22 @@ list lazy_list_set(list L, integer i, list v)
ret.ch.append(self.Parse_expression()) ret.ch.append(self.Parse_expression())
self.expect(')') self.expect(')')
self.NextToken() self.NextToken()
saveSuspiciousStSw = self.SuspiciousStSw savePruneBug = self.PruneBug
self.SuspiciousStSw = [] self.PruneBug = []
ret.ch.append(self.Parse_statement(ReturnType, AllowStSw = None, InsideLoop = InsideLoop)) ret.ch.append(self.Parse_statement(ReturnType, AllowStSw = None, InsideLoop = InsideLoop))
if self.tok[0] == 'ELSE': if self.tok[0] == 'ELSE':
if AllowStSw is False and self.SuspiciousStSw: if AllowStSw is False and self.PruneBug:
self.errorpos = self.SuspiciousStSw[0] self.errorpos = self.PruneBug[0][0]
raise EParseCantChangeState(self) raise self.PruneBug[0][1](self)
LastIsReturn = getattr(ret.ch[1], 'LIR', False) LastIsReturn = getattr(ret.ch[1], 'LIR', False)
self.NextToken() self.NextToken()
ret.ch.append(self.Parse_statement(ReturnType, ret.ch.append(self.Parse_statement(ReturnType,
AllowStSw = AllowStSw, InsideLoop = InsideLoop)) AllowStSw = AllowStSw, InsideLoop = InsideLoop))
if AllowStSw is None: if AllowStSw is None:
saveSuspiciousStSw += self.SuspiciousStSw savePruneBug += self.PruneBug
if LastIsReturn and getattr(ret.ch[2], 'LIR', False): if LastIsReturn and getattr(ret.ch[2], 'LIR', False):
ret.LIR = True ret.LIR = True
self.SuspiciousStSw = saveSuspiciousStSw self.PruneBug = savePruneBug
return ret return ret
if tok0 == 'WHILE': if tok0 == 'WHILE':
@ -2915,8 +2935,10 @@ list lazy_list_set(list L, integer i, list v)
# List of preprocessor #line directives. # List of preprocessor #line directives.
self.linedir = [] self.linedir = []
# List of positions with suspicious state change statements. # List of tuples (position, exception) where suspicious state change
self.SuspiciousStSw = [] # statements or returns with void expressions happen. These can only
# be detected when the 'else' is found.
self.PruneBug = []
# This is a small hack to prevent circular definitions in globals when # This is a small hack to prevent circular definitions in globals when
# extended expressions are enabled. When false (default), forward # extended expressions are enabled. When false (default), forward

View file

@ -368,8 +368,19 @@ class UnitTestRegression(UnitTestCase):
parser.parse('default{timer(){[llDie()];}}') parser.parse('default{timer(){[llDie()];}}')
parser.parse('default{timer(){llDie();}}') parser.parse('default{timer(){llDie();}}')
parser.parse('default{timer(){(llDie());}}') parser.parse('default{timer(){(llDie());}}')
parser.parse('default{timer(){for(llDie();1;llDie());}}' parser.parse('default{timer(){for(llDie();1;llDie());}}',
, ('optimize',)) ('optimize',))
# 'return <void expr>' works in the same situations as state changes
self.assertRaises(lslparse.EParseReturnShouldBeEmpty, parser.parse,
'default{timer(){return llDie();}}')
self.assertRaises(lslparse.EParseReturnShouldBeEmpty, parser.parse,
'default{timer(){if(1)return llDie();else;}}')
self.assertRaises(lslparse.EParseReturnShouldBeEmpty, parser.parse,
'default{timer(){if(1);else return llDie();}}')
self.assertRaises(lslparse.EParseReturnShouldBeEmpty, parser.parse,
'default{timer(){return 1;}}')
self.assertRaises(lslparse.EParseTypeMismatch, parser.parse,
'default{timer(){if(1)return 1;}}')
class UnitTestCoverage(UnitTestCase): class UnitTestCoverage(UnitTestCase):
def test_coverage_misc(self): def test_coverage_misc(self):

View file

@ -0,0 +1,41 @@
x(){
// All these should work
if (1)
return llDie();
while (1)
return llDie();
do
return llDie();
while (1);
for (;1;)
return llDie();
if (1)
{
return llDie();
if (1) return llDie(); else return llDie();
return llDie();
}
}
default
{
state_entry()
{
// Similarly, all these should work
if (1)
return llDie();
while (1)
return llDie();
do
return llDie();
while (1);
for (;1;)
return llDie();
if (1)
{
return llDie();
if (1) return llDie(); else return llDie();
return llDie();
}
}
}

View file

@ -0,0 +1,94 @@
x()
{
if (1)
{
llDie();
return;
}
while (1)
{
llDie();
return;
}
do
{
llDie();
return;
}
while (1);
for (; 1; )
{
llDie();
return;
}
if (1)
{
{
llDie();
return;
}
if (1)
{
llDie();
return;
}
else
{
llDie();
return;
}
{
llDie();
return;
}
}
}
default
{
state_entry()
{
if (1)
{
llDie();
return;
}
while (1)
{
llDie();
return;
}
do
{
llDie();
return;
}
while (1);
for (; 1; )
{
llDie();
return;
}
if (1)
{
{
llDie();
return;
}
if (1)
{
llDie();
return;
}
else
{
llDie();
return;
}
{
llDie();
return;
}
}
}
}

View file

@ -0,0 +1 @@
main.py - -O -optimize