Skip to content

A
angle
Commit 9a1b49f7 by Jamie Madill
Options

Revert "Don't evaluate short-circuited preprocessor expressions"

Build break on Clang: FAILED: /b/build/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/third_party/angle/src/compiler/preprocessor/preprocessor.ExpressionParser.o.d -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -DCHROMIUM_BUILD -DCR_CLANG_REVISION=245965-1 -DUSE_LIBJPEG_TURBO=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_PRE_SYNC_BACKUP -DENABLE_WEBRTC=1 -DENABLE_MEDIA_ROUTER=1 -DUSE_PROPRIETARY_CODECS -DENABLE_PEPPER_CDMS -DENABLE_CONFIGURATION_POLICY -DENABLE_NOTIFICATIONS -DENABLE_HIDPI=1 -DSYSTEM_NATIVELY_SIGNALS_MEMORY_PRESSURE -DDONT_EMBED_BUILD_METADATA -DDCHECK_ALWAYS_ON=1 -DFIELDTRIAL_TESTING_ENABLED -DENABLE_TASK_MANAGER=1 -DENABLE_EXTENSIONS=1 -DENABLE_PDF=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PLUGINS=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_AUTOFILL_DIALOG=1 -DENABLE_BACKGROUND=1 -DENABLE_GOOGLE_NOW=1 -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 -DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_APP_LIST=1 -DENABLE_SETTINGS_APP=1 -DENABLE_SUPERVISED_USERS=1 -DENABLE_SERVICE_DISCOVERY=1 -DENABLE_WIFI_BOOTSTRAPPING=1 -DV8_USE_EXTERNAL_STARTUP_DATA -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DSAFE_BROWSING_SERVICE -DUSE_LIBPCI=1 -DUSE_OPENSSL=1 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -Igen -isysroot /Applications/Xcode5.1.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk -O2 -gdwarf-2 -fvisibility=hidden -Werror -Wnewline-eof -mmacosx-version-min=10.6 -arch x86_64 -Wendif-labels -Wno-unused-parameter -Wno-missing-field-initializers -Wno-selector-type-mismatch -Wpartial-availability -Wheader-hygiene -Wno-char-subscripts -Wno-unneeded-internal-declaration -Wno-covered-switch-default -Wstring-conversion -Wno-c++11-narrowing -Wno-deprecated-register -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-unused-function -Wno-unused-variable -std=c++11 -fno-rtti -fno-exceptions -fvisibility-inlines-hidden -fno-threadsafe-statics -Xclang -load -Xclang /b/build/slave/GPU_Mac_Builder/build/src/third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-templates -fcolor-diagnostics -fno-strict-aliasing -c ../../third_party/angle/src/compiler/preprocessor/ExpressionParser.cpp -o obj/third_party/angle/src/compiler/preprocessor/preprocessor.ExpressionParser.o ../../third_party/angle/src/compiler/preprocessor/ExpressionParser.cpp:1372:35: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand] (yyval) = (yyvsp[-3]) || 0; ^ ~ ../../third_party/angle/src/compiler/preprocessor/ExpressionParser.cpp:1372:35: note: use '|' for a bitwise operation (yyval) = (yyvsp[-3]) || 0; ^~ | ../../third_party/angle/src/compiler/preprocessor/ExpressionParser.cpp:1406:35: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand] (yyval) = (yyvsp[-3]) && 0; ^ ~ ../../third_party/angle/src/compiler/preprocessor/ExpressionParser.cpp:1406:35: note: use '&' for a bitwise operation (yyval) = (yyvsp[-3]) && 0; ^~ & ../../third_party/angle/src/compiler/preprocessor/ExpressionParser.cpp:1406:35: note: remove constant to silence this warning (yyval) = (yyvsp[-3]) && 0; ~^~~~ BUG=angleproject:347 This reverts commit 6ffe6135. Change-Id: I6d81666cca573f320bfb1164a6c794b6f75f7463 Reviewed-on: https://chromium-review.googlesource.com/298020Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Tested-by: 's avatarJamie Madill <jmadill@chromium.org>
parent 47f730fa
Showing with 27 additions and 168 deletions
src/compiler/preprocessor/ExpressionParser.y
...@@ -64,13 +64,6 @@ struct Context ...@@ -64,13 +64,6 @@ struct Context
pp::Lexer* lexer; pp::Lexer* lexer;
pp::Token* token; pp::Token* token;
int* result; int* result;
void startIgnoreErrors() { ++ignoreErrors; }
void endIgnoreErrors() { --ignoreErrors; }
bool isIgnoringErrors() { return ignoreErrors > 0; }
int ignoreErrors;
}; };
} // namespace } // namespace
%} %}
...@@ -86,7 +79,6 @@ static void yyerror(Context* context, const char* reason); ...@@ -86,7 +79,6 @@ static void yyerror(Context* context, const char* reason);
%} %}
%token TOK_CONST_INT %token TOK_CONST_INT
%token TOK_IDENTIFIER
%left TOK_OP_OR %left TOK_OP_OR
%left TOK_OP_AND %left TOK_OP_AND
%left '|' %left '|'
...@@ -104,62 +96,17 @@ static void yyerror(Context* context, const char* reason); ...@@ -104,62 +96,17 @@ static void yyerror(Context* context, const char* reason);
input input
: expression { : expression {
*(context->result) = static_cast<int>($1); *(context->result) = static_cast<int>($1);
YYACCEPT;
} }
; ;
expression expression
: TOK_CONST_INT : TOK_CONST_INT
| TOK_IDENTIFIER { | expression TOK_OP_OR expression {
if (!context->isIgnoringErrors()) $$ = $1 || $3;
{
YYABORT;
}
} }
| expression TOK_OP_OR { | expression TOK_OP_AND expression {
if ($1 != 0) $$ = $1 && $3;
{
// Ignore errors in the short-circuited part of the expression.
// ESSL3.00 section 3.4:
// If an operand is not evaluated, the presence of undefined identifiers
// in the operand will not cause an error.
// Unevaluated division by zero should not cause an error either.
context->startIgnoreErrors();
}
} expression {
if ($1 != 0)
{
context->endIgnoreErrors();
// Make sure that short-circuited version of || works consistently
// with the normal version even for values other than 0 and 1.
$$ = $1 || 0;
}
else
{
$$ = $1 || $4;
}
}
| expression TOK_OP_AND {
if ($1 == 0)
{
// Ignore errors in the short-circuited part of the expression.
// ESSL3.00 section 3.4:
// If an operand is not evaluated, the presence of undefined identifiers
// in the operand will not cause an error.
// Unevaluated division by zero should not cause an error either.
context->startIgnoreErrors();
}
} expression {
if ($1 == 0)
{
context->endIgnoreErrors();
// Make sure that short-circuited version of && works consistently
// with the normal version even for values other than 0 and 1.
$$ = $1 && 0;
}
else
{
$$ = $1 && $4;
}
} }
| expression '|' expression { | expression '|' expression {
$$ = $1 | $3; $$ = $1 | $3;
...@@ -201,48 +148,28 @@ expression ...@@ -201,48 +148,28 @@ expression
$$ = $1 + $3; $$ = $1 + $3;
} }
| expression '%' expression { | expression '%' expression {
if ($3 == 0) if ($3 == 0) {
{ std::ostringstream stream;
if (context->isIgnoringErrors()) stream << $1 << " % " << $3;
{ std::string text = stream.str();
$$ = static_cast<YYSTYPE>(0); context->diagnostics->report(pp::Diagnostics::PP_DIVISION_BY_ZERO,
} context->token->location,
else text.c_str());
{ YYABORT;
std::ostringstream stream; } else {
stream << $1 << " % " << $3;
std::string text = stream.str();
context->diagnostics->report(pp::Diagnostics::PP_DIVISION_BY_ZERO,
context->token->location,
text.c_str());
YYABORT;
}
}
else
{
$$ = $1 % $3; $$ = $1 % $3;
} }
} }
| expression '/' expression { | expression '/' expression {
if ($3 == 0) if ($3 == 0) {
{ std::ostringstream stream;
if (context->isIgnoringErrors()) stream << $1 << " / " << $3;
{ std::string text = stream.str();
$$ = static_cast<YYSTYPE>(0); context->diagnostics->report(pp::Diagnostics::PP_DIVISION_BY_ZERO,
} context->token->location,
else text.c_str());
{ YYABORT;
std::ostringstream stream; } else {
stream << $1 << " / " << $3;
std::string text = stream.str();
context->diagnostics->report(pp::Diagnostics::PP_DIVISION_BY_ZERO,
context->token->location,
text.c_str());
YYABORT;
}
}
else
{
$$ = $1 / $3; $$ = $1 / $3;
} }
} }
...@@ -286,15 +213,6 @@ int yylex(YYSTYPE *lvalp, Context *context) ...@@ -286,15 +213,6 @@ int yylex(YYSTYPE *lvalp, Context *context)
type = TOK_CONST_INT; type = TOK_CONST_INT;
break; break;
} }
case pp::Token::IDENTIFIER:
if (!context->isIgnoringErrors())
{
context->diagnostics->report(pp::Diagnostics::PP_CONDITIONAL_UNEXPECTED_TOKEN,
token->location, token->text);
}
*lvalp = static_cast<YYSTYPE>(-1);
type = TOK_IDENTIFIER;
break;
case pp::Token::OP_OR: case pp::Token::OP_OR:
type = TOK_OP_OR; type = TOK_OP_OR;
break; break;
...@@ -369,7 +287,6 @@ bool ExpressionParser::parse(Token *token, int *result) ...@@ -369,7 +287,6 @@ bool ExpressionParser::parse(Token *token, int *result)
context.lexer = mLexer; context.lexer = mLexer;
context.token = token; context.token = token;
context.result = result; context.result = result;
context.ignoreErrors = 0;
int ret = yyparse(&context); int ret = yyparse(&context);
switch (ret) switch (ret)
{ {
......
src/tests/preprocessor_tests/if_test.cpp
...@@ -697,6 +697,10 @@ TEST_F(IfTest, UndefinedMacro) ...@@ -697,6 +697,10 @@ TEST_F(IfTest, UndefinedMacro)
ASSERT_TRUE(mPreprocessor.init(1, &str, 0)); ASSERT_TRUE(mPreprocessor.init(1, &str, 0));
EXPECT_CALL(mDiagnostics, EXPECT_CALL(mDiagnostics,
print(pp::Diagnostics::PP_INVALID_EXPRESSION,
pp::SourceLocation(0, 1),
"syntax error"));
EXPECT_CALL(mDiagnostics,
print(pp::Diagnostics::PP_CONDITIONAL_UNEXPECTED_TOKEN, print(pp::Diagnostics::PP_CONDITIONAL_UNEXPECTED_TOKEN,
pp::SourceLocation(0, 1), pp::SourceLocation(0, 1),
"UNDEFINED")); "UNDEFINED"));
...@@ -829,65 +833,3 @@ TEST_F(IfTest, UnterminatedIfdef) ...@@ -829,65 +833,3 @@ TEST_F(IfTest, UnterminatedIfdef)
mPreprocessor.lex(&token); mPreprocessor.lex(&token);
} }
// The preprocessor only allows one expression to follow an #if directive.
// Supplying two integer expressions should be an error.
TEST_F(IfTest, ExtraIntExpression)
{
const char *str =
"#if 1 1\n"
"#endif\n";
ASSERT_TRUE(mPreprocessor.init(1, &str, 0));
EXPECT_CALL(mDiagnostics, print(pp::Diagnostics::PP_INVALID_EXPRESSION,
pp::SourceLocation(0, 1), "syntax error"));
pp::Token token;
mPreprocessor.lex(&token);
}
// The preprocessor only allows one expression to follow an #if directive.
// Supplying two expressions where one uses a preprocessor define should be an error.
TEST_F(IfTest, ExtraIdentifierExpression)
{
const char *str =
"#define one 1\n"
"#if 1 one\n"
"#endif\n";
ASSERT_TRUE(mPreprocessor.init(1, &str, 0));
EXPECT_CALL(mDiagnostics, print(pp::Diagnostics::PP_INVALID_EXPRESSION,
pp::SourceLocation(0, 2), "syntax error"));
pp::Token token;
mPreprocessor.lex(&token);
}
// Divide by zero that's not evaluated because of short-circuiting should not cause an error.
TEST_F(IfTest, ShortCircuitedDivideByZero)
{
const char *str =
"#if 1 || (2 / 0)\n"
"pass\n"
"#endif\n";
const char *expected =
"\n"
"pass\n"
"\n";
preprocess(str, expected);
}
// Undefined identifier that's not evaluated because of short-circuiting should not cause an error.
TEST_F(IfTest, ShortCircuitedUndefined)
{
const char *str =
"#if 1 || UNDEFINED\n"
"pass\n"
"#endif\n";
const char *expected =
"\n"
"pass\n"
"\n";
preprocess(str, 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