Commit e6432c85 by Olli Etuaho

Fix preprocessor macro replacement list location

According to the dEQP tests, a macro replacement list generated by a function-like macro invocation should get its location from the closing parenthesis of the invocation. The tests check this by using __LINE__ in a macro with a multi-line invocation. It's not quite clear from the spec that the enforced behavior is expected as opposed to the replacement list getting its location from the macro name, but a minor correction to the preprocessor makes the dEQP tests pass. Newlines in the preprocessor unit tests are generated according to the source locations in the token list produced by the preprocessor, so the expectations of a few tests also need to be updated. BUG=angleproject:989 TEST=dEQP-GLES3.functional.shaders.preprocessor.predefined_macros.* (2 start passing with this change), angle_unittests Change-Id: I4cc9da09bd0985310a05ebf6def680916a46308a Reviewed-on: https://chromium-review.googlesource.com/297990Tested-by: 's avatarOlli Etuaho <oetuaho@nvidia.com> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarZhenyao Mo <zmo@chromium.org>
parent 0304d2fe
...@@ -187,6 +187,12 @@ bool MacroExpander::expandMacro(const Macro &macro, ...@@ -187,6 +187,12 @@ bool MacroExpander::expandMacro(const Macro &macro,
std::vector<Token> *replacements) std::vector<Token> *replacements)
{ {
replacements->clear(); replacements->clear();
// In the case of an object-like macro, the replacement list gets its location
// from the identifier, but in the case of a function-like macro, the replacement
// list gets its location from the closing parenthesis of the macro invocation.
// This is tested by dEQP-GLES3.functional.shaders.preprocessor.predefined_macros.*
SourceLocation replacementLocation = identifier.location;
if (macro.type == Macro::kTypeObj) if (macro.type == Macro::kTypeObj)
{ {
replacements->assign(macro.replacements.begin(), replacements->assign(macro.replacements.begin(),
...@@ -218,7 +224,7 @@ bool MacroExpander::expandMacro(const Macro &macro, ...@@ -218,7 +224,7 @@ bool MacroExpander::expandMacro(const Macro &macro,
assert(macro.type == Macro::kTypeFunc); assert(macro.type == Macro::kTypeFunc);
std::vector<MacroArg> args; std::vector<MacroArg> args;
args.reserve(macro.parameters.size()); args.reserve(macro.parameters.size());
if (!collectMacroArgs(macro, identifier, &args)) if (!collectMacroArgs(macro, identifier, &args, &replacementLocation))
return false; return false;
replaceMacroParams(macro, args, replacements); replaceMacroParams(macro, args, replacements);
...@@ -234,14 +240,15 @@ bool MacroExpander::expandMacro(const Macro &macro, ...@@ -234,14 +240,15 @@ bool MacroExpander::expandMacro(const Macro &macro,
repl.setAtStartOfLine(identifier.atStartOfLine()); repl.setAtStartOfLine(identifier.atStartOfLine());
repl.setHasLeadingSpace(identifier.hasLeadingSpace()); repl.setHasLeadingSpace(identifier.hasLeadingSpace());
} }
repl.location = identifier.location; repl.location = replacementLocation;
} }
return true; return true;
} }
bool MacroExpander::collectMacroArgs(const Macro &macro, bool MacroExpander::collectMacroArgs(const Macro &macro,
const Token &identifier, const Token &identifier,
std::vector<MacroArg> *args) std::vector<MacroArg> *args,
SourceLocation *closingParenthesisLocation)
{ {
Token token; Token token;
getToken(&token); getToken(&token);
...@@ -271,6 +278,7 @@ bool MacroExpander::collectMacroArgs(const Macro &macro, ...@@ -271,6 +278,7 @@ bool MacroExpander::collectMacroArgs(const Macro &macro,
case ')': case ')':
--openParens; --openParens;
isArg = openParens != 0; isArg = openParens != 0;
*closingParenthesisLocation = token.location;
break; break;
case ',': case ',':
// The individual arguments are separated by comma tokens, but // The individual arguments are separated by comma tokens, but
......
...@@ -19,6 +19,7 @@ namespace pp ...@@ -19,6 +19,7 @@ namespace pp
{ {
class Diagnostics; class Diagnostics;
struct SourceLocation;
class MacroExpander : public Lexer class MacroExpander : public Lexer
{ {
...@@ -45,7 +46,8 @@ class MacroExpander : public Lexer ...@@ -45,7 +46,8 @@ class MacroExpander : public Lexer
typedef std::vector<Token> MacroArg; typedef std::vector<Token> MacroArg;
bool collectMacroArgs(const Macro &macro, bool collectMacroArgs(const Macro &macro,
const Token &identifier, const Token &identifier,
std::vector<MacroArg> *args); std::vector<MacroArg> *args,
SourceLocation *closingParenthesisLocation);
void replaceMacroParams(const Macro &macro, void replaceMacroParams(const Macro &macro,
const std::vector<MacroArg> &args, const std::vector<MacroArg> &args,
std::vector<Token> *replacements); std::vector<Token> *replacements);
......
...@@ -454,10 +454,10 @@ TEST_F(DefineTest, FuncExtraNewlines) ...@@ -454,10 +454,10 @@ TEST_F(DefineTest, FuncExtraNewlines)
"1\n" "1\n"
")\n"; ")\n";
const char* expected = "\n" const char* expected = "\n"
"(1)\n"
"\n" "\n"
"\n" "\n"
"\n"; "\n"
"(1)\n";
preprocess(input, expected); preprocess(input, expected);
} }
...@@ -789,6 +789,7 @@ TEST_F(DefineTest, UndefRedefine) ...@@ -789,6 +789,7 @@ TEST_F(DefineTest, UndefRedefine)
preprocess(input, expected); preprocess(input, expected);
} }
// Example from C99 standard section 6.10.3.5 Scope of macro definitions
TEST_F(DefineTest, C99Example) TEST_F(DefineTest, C99Example)
{ {
const char* input = const char* input =
...@@ -824,8 +825,8 @@ TEST_F(DefineTest, C99Example) ...@@ -824,8 +825,8 @@ TEST_F(DefineTest, C99Example)
"\n" "\n"
"\n" "\n"
"f(2 * (y+1)) + f(2 * (f(2 * (z[0])))) % f(2 * (0)) + t(1);\n" "f(2 * (y+1)) + f(2 * (f(2 * (z[0])))) % f(2 * (0)) + t(1);\n"
"f(2 * (2+(3,4)-0,1)) | f(2 * (~ 5)) & f(2 * (0,1))\n" "f(2 * (2+(3,4)-0,1)) | f(2 * (~ 5)) &\n"
"^m(0,1);\n" " f(2 * (0,1))^m(0,1);\n"
"int i[] = { 1, 23, 4, 5, };\n"; "int i[] = { 1, 23, 4, 5, };\n";
preprocess(input, expected); preprocess(input, expected);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment