commit d7cd4ae5c2f71ff3acc51997a2b816839f4f5947
author Olli Etuaho <oetuaho@nvidia.com> Thu Jul 06 15:52:49 2017 +0300
committer Commit Bot <commit-bot@chromium.org> Thu Jul 06 16:19:24 2017 +0000
tree 4fb970241099f63188801f3dd41d97fe95c74dd2
parent 40a1a37c61e63bbd58144124f8f694ea145c7227

Check that function declarations don't use a reserved name

Reserved function names are now caught if the function is just
declared without being called in the shader source. Actually, function
calls don't need to be checked for reserved names, since that just
generates a redundant error message if function declarations are being
checked.

Includes some cleanup of ParseContext::checkIsNotReserved. It doesn't
need special handling of built-in symbols, as they are never passed to
the function.

BUG=chromium:739448
TEST=angle_unittests

Change-Id: I7115e1a7509626b5109b5c054c0704b0c3c19c58
Reviewed-on: https://chromium-review.googlesource.com/561457
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>

src/compiler/translator/ParseContext.cpp

diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 975fade..c1e8f3e 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -563,44 +563,38 @@
     return true;
 }
 
-// For now, keep it simple:  if it starts "gl_", it's reserved, independent
-// of scope.  Except, if the symbol table is at the built-in push-level,
-// which is when we are parsing built-ins.
-// Also checks for "webgl_" and "_webgl_" reserved identifiers if parsing a
-// webgl shader.
+// ESSL 3.00.5 sections 3.8 and 3.9.
+// If it starts "gl_" or contains two consecutive underscores, it's reserved.
+// Also checks for "webgl_" and "_webgl_" reserved identifiers if parsing a webgl shader.
 bool TParseContext::checkIsNotReserved(const TSourceLoc &line, const TString &identifier)
 {
     static const char *reservedErrMsg = "reserved built-in name";
-    if (!symbolTable.atBuiltInLevel())
+    if (identifier.compare(0, 3, "gl_") == 0)
     {
-        if (identifier.compare(0, 3, "gl_") == 0)
+        error(line, reservedErrMsg, "gl_");
+        return false;
+    }
+    if (sh::IsWebGLBasedSpec(mShaderSpec))
+    {
+        if (identifier.compare(0, 6, "webgl_") == 0)
         {
-            error(line, reservedErrMsg, "gl_");
+            error(line, reservedErrMsg, "webgl_");
             return false;
         }
-        if (sh::IsWebGLBasedSpec(mShaderSpec))
+        if (identifier.compare(0, 7, "_webgl_") == 0)
         {
-            if (identifier.compare(0, 6, "webgl_") == 0)
-            {
-                error(line, reservedErrMsg, "webgl_");
-                return false;
-            }
-            if (identifier.compare(0, 7, "_webgl_") == 0)
-            {
-                error(line, reservedErrMsg, "_webgl_");
-                return false;
-            }
-        }
-        if (identifier.find("__") != TString::npos)
-        {
-            error(line,
-                  "identifiers containing two consecutive underscores (__) are reserved as "
-                  "possible future keywords",
-                  identifier.c_str());
+            error(line, reservedErrMsg, "_webgl_");
             return false;
         }
     }
-
+    if (identifier.find("__") != TString::npos)
+    {
+        error(line,
+              "identifiers containing two consecutive underscores (__) are reserved as "
+              "possible future keywords",
+              identifier.c_str());
+        return false;
+    }
     return true;
 }
 
@@ -2737,6 +2731,8 @@
     const TSourceLoc &location,
     bool insertParametersToSymbolTable)
 {
+    checkIsNotReserved(location, function.getName());
+
     TIntermFunctionPrototype *prototype =
         new TIntermFunctionPrototype(function.getReturnType(), TSymbolUniqueId(function));
     // TODO(oetuaho@nvidia.com): Instead of converting the function information here, the node could
@@ -2997,7 +2993,6 @@
 
 TFunction *TParseContext::addNonConstructorFunc(const TString *name, const TSourceLoc &loc)
 {
-    checkIsNotReserved(loc, *name);
     const TType *returnType = TCache::getType(EbtVoid, EbpUndefined);
     return new TFunction(name, returnType);
 }