Allow limited form of expressions in #line directives
Reuse ExpressionParser that's also used for parsing preprocessor
conditionals to parse line and file numbers in #line directives.
According to recent Khronos discussions, the intent of the spec is that
expressions in #line directives should be interpreted similarly to
expressions in conditional directives, so reusing ExpressionParser is a
natural way to implement this. This enables simple math operators
operating on integers. There are a few unclear corner cases, but this
approach is enough to support practical use cases and pass the dEQP
tests.
Valid line directives have one of the following forms:
#line line-expression
#line line-expression file-expression
ExpressionParser is first run to parse the line-expression. In ambiguous
cases the ExpressionParser consumes as much of the line as possible to
form line-expression. Then, if end-of-line hasn't been reached,
file-expression is parsed by running ExpressionParser again. As an
example of an ambiguous case:
#line 1 + 2
This could alternatively be interpreted to mean line-expression "1" and
file-expression "+ 2" where + is the unary + operator, but ANGLE now
interprets it to mean line-expression "1 + 2". Because of these ambiguous
cases, a bison grammar that would parse multiple expressions on the same
line couldn't be easily constructed, so this solution where
ExpressionParser is run twice was chosen instead.
The problematic corner cases are:
- ExpressionParser uses 64-bit integers internally for evaluating the
expression's value. It's possible to interpret the ESSL3 spec so that
32-bit integer wraparound behavior would be required also for #line
directive expressions.
- It's unclear whether the defined operator can be used in #line
expressions. In this patch it is disabled. Hoping for further
clarification from Khronos.
- It's unclear how short-circuiting should affect the parsing of
undefined identifiers in #line expressions. Now it's consistent with #if
expressions (undefined identifiers are OK if they're short-circuited).
dEQP expectations are updated for preprocessor tests, including ones
not affected specifically by this change.
BUG=angleproject:989
TEST=angle_unittests,
dEQP-GLES3.functional.shaders.preprocessor.* (4 start passing),
dEQP-GLES2.functional.shaders.preprocessor.* (4 start passing)
Change-Id: I55c5bf75857da5de855cc600d3603ee19399f328
Reviewed-on: https://chromium-review.googlesource.com/300964
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Tryjob-Request: Olli Etuaho <oetuaho@nvidia.com>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/preprocessor/DirectiveParser.cpp b/src/compiler/preprocessor/DirectiveParser.cpp
index 6da84bd..203630a 100644
--- a/src/compiler/preprocessor/DirectiveParser.cpp
+++ b/src/compiler/preprocessor/DirectiveParser.cpp
@@ -764,72 +764,60 @@
{
assert(getDirective(token) == DIRECTIVE_LINE);
- enum State
- {
- LINE_NUMBER,
- FILE_NUMBER
- };
-
bool valid = true;
+ bool parsedFileNumber = false;
int line = 0, file = 0;
- int state = LINE_NUMBER;
MacroExpander macroExpander(mTokenizer, mMacroSet, mDiagnostics, false);
+
+ // Lex the first token after "#line" so we can check it for EOD.
macroExpander.lex(token);
- while ((token->type != '\n') && (token->type != Token::LAST))
+
+ if (isEOD(token))
{
- switch (state++)
+ mDiagnostics->report(Diagnostics::PP_INVALID_LINE_DIRECTIVE, token->location, token->text);
+ valid = false;
+ }
+ else
+ {
+ ExpressionParser expressionParser(¯oExpander, mDiagnostics);
+ ExpressionParser::ErrorSettings errorSettings;
+
+ // See GLES3 section 12.42
+ errorSettings.integerLiteralsMustFit32BitSignedRange = true;
+
+ errorSettings.unexpectedIdentifier = Diagnostics::PP_INVALID_LINE_NUMBER;
+ // The first token was lexed earlier to check if it was EOD. Include
+ // the token in parsing for a second time by setting the
+ // parsePresetToken flag to true.
+ expressionParser.parse(token, &line, true, errorSettings, &valid);
+ if (!isEOD(token) && valid)
{
- case LINE_NUMBER:
- if (valid && (token->type != Token::CONST_INT))
- {
- mDiagnostics->report(Diagnostics::PP_INVALID_LINE_NUMBER,
- token->location, token->text);
- valid = false;
- }
- if (valid && !token->iValue(&line))
- {
- mDiagnostics->report(Diagnostics::PP_INTEGER_OVERFLOW,
- token->location, token->text);
- valid = false;
- }
- break;
- case FILE_NUMBER:
- if (valid && (token->type != Token::CONST_INT))
- {
- mDiagnostics->report(Diagnostics::PP_INVALID_FILE_NUMBER,
- token->location, token->text);
- valid = false;
- }
- if (valid && !token->iValue(&file))
- {
- mDiagnostics->report(Diagnostics::PP_INTEGER_OVERFLOW,
- token->location, token->text);
- valid = false;
- }
- break;
- default:
+ errorSettings.unexpectedIdentifier = Diagnostics::PP_INVALID_FILE_NUMBER;
+ // After parsing the line expression expressionParser has also
+ // advanced to the first token of the file expression - this is the
+ // token that makes the parser reduce the "input" rule for the line
+ // expression and stop. So we're using parsePresetToken = true here
+ // as well.
+ expressionParser.parse(token, &file, true, errorSettings, &valid);
+ parsedFileNumber = true;
+ }
+ if (!isEOD(token))
+ {
if (valid)
{
mDiagnostics->report(Diagnostics::PP_UNEXPECTED_TOKEN,
token->location, token->text);
valid = false;
}
- break;
+ skipUntilEOD(mTokenizer, token);
}
- macroExpander.lex(token);
}
- if (valid && (state != FILE_NUMBER) && (state != FILE_NUMBER + 1))
- {
- mDiagnostics->report(Diagnostics::PP_INVALID_LINE_DIRECTIVE,
- token->location, token->text);
- valid = false;
- }
if (valid)
{
mTokenizer->setLineNumber(line);
- if (state == FILE_NUMBER + 1)
+ if (parsedFileNumber)
mTokenizer->setFileNumber(file);
}
}
@@ -893,7 +881,12 @@
ExpressionParser expressionParser(¯oExpander, mDiagnostics);
int expression = 0;
- expressionParser.parse(token, &expression);
+ ExpressionParser::ErrorSettings errorSettings;
+ errorSettings.integerLiteralsMustFit32BitSignedRange = false;
+ errorSettings.unexpectedIdentifier = Diagnostics::PP_CONDITIONAL_UNEXPECTED_TOKEN;
+
+ bool valid = true;
+ expressionParser.parse(token, &expression, false, errorSettings, &valid);
// Check if there are tokens after #if expression.
if (!isEOD(token))