Просмотр исходного кода

Revert "Fix erroneous parse error when declaring variables using struct names" (#4002)

* Revert "Remove trailing whitespaces - NFC"

This reverts commit 15b727afdd0b69ef5d178727e7ac66be991e678f.

* Revert "Fix while avoiding grammar changes"

This reverts commit a2b6c9183b8293a4dc41cc52a03095f0d1ed9257.

* Revert "Fix spec violations. Simplify tests."

This reverts commit fa7cb74569749b637678aac31971f6ab08f4e707.

* Revert "Fix erroneous parse error when declaring variables using struct names"

This reverts commit cefdfc3b63d178e1e163ebd5cd927dbfb11bb261.
Diego Novillo 5 дней назад
Родитель
Сommit
f43df42fe6

+ 0 - 14
Test/struct_name_as_array_member.frag

@@ -1,14 +0,0 @@
-#version 400
-
-// Test case for using a struct name as an array member name.
-struct B
-{
-    vec3 t;
-};
-
-struct K
-{
-    float A[2], B[3]; // This should work - struct name B is used as an array member name.
-};
-
-void main(){}

+ 0 - 20
Test/struct_name_as_struct_member.frag

@@ -1,20 +0,0 @@
-#version 400
-
-// Original bug report from: https://github.com/KhronosGroup/glslang/issues/3931
-// Error was: "syntax error, unexpected TYPE_NAME, expecting IDENTIFIER"
-// when defining a struct field with the same name as a previously defined
-// struct.
-struct B
-{
-    vec3 t;
-};
-
-struct K
-{
-    float A, B; // This should work. The B field is in a different scope
-		// than the B struct.
-};
-
-void main(){
-  int x, B, y, K;
-}

+ 12 - 34
glslang/MachineIndependent/Scan.cpp

@@ -836,21 +836,12 @@ int TScanContext::tokenize(TPpContext* pp, TParserToken& token)
         loc = ppToken.loc;
         loc = ppToken.loc;
         parserToken->sType.lex.loc = loc;
         parserToken->sType.lex.loc = loc;
         switch (token) {
         switch (token) {
-        case ';':  afterType = false; afterBuffer = false; inDeclaratorList = false; afterDeclarator = false; angleBracketDepth = 0; squareBracketDepth = 0; return SEMICOLON;
-        case ',':
-            // If we just processed a declarator (identifier after a type), this comma
-            // indicates that we're in a declarator list. Note that 'afterDeclarator' is
-            // only set when we are not inside a template parameter list or array expression.
-            if (afterDeclarator) {
-                inDeclaratorList = true;
-            }
-            afterType = false;
-            afterDeclarator = false;
-            return COMMA;
+        case ';':  afterType = false; afterBuffer = false; return SEMICOLON;
+        case ',':  afterType = false;   return COMMA;
         case ':':                       return COLON;
         case ':':                       return COLON;
-        case '=':  afterType = false; inDeclaratorList = false; afterDeclarator = false;   return EQUAL;
-        case '(':  afterType = false; inDeclaratorList = false; afterDeclarator = false;   return LEFT_PAREN;
-        case ')':  afterType = false; inDeclaratorList = false; afterDeclarator = false;   return RIGHT_PAREN;
+        case '=':  afterType = false;   return EQUAL;
+        case '(':  afterType = false;   return LEFT_PAREN;
+        case ')':  afterType = false;   return RIGHT_PAREN;
         case '.':  field = true;        return DOT;
         case '.':  field = true;        return DOT;
         case '!':                       return BANG;
         case '!':                       return BANG;
         case '-':                       return DASH;
         case '-':                       return DASH;
@@ -859,16 +850,16 @@ int TScanContext::tokenize(TPpContext* pp, TParserToken& token)
         case '*':                       return STAR;
         case '*':                       return STAR;
         case '/':                       return SLASH;
         case '/':                       return SLASH;
         case '%':                       return PERCENT;
         case '%':                       return PERCENT;
-        case '<':                       angleBracketDepth++; return LEFT_ANGLE;
-        case '>':                       if (angleBracketDepth > 0) angleBracketDepth--; return RIGHT_ANGLE;
+        case '<':                       return LEFT_ANGLE;
+        case '>':                       return RIGHT_ANGLE;
         case '|':                       return VERTICAL_BAR;
         case '|':                       return VERTICAL_BAR;
         case '^':                       return CARET;
         case '^':                       return CARET;
         case '&':                       return AMPERSAND;
         case '&':                       return AMPERSAND;
         case '?':                       return QUESTION;
         case '?':                       return QUESTION;
-        case '[':                       squareBracketDepth++; return LEFT_BRACKET;
-        case ']':                       if (squareBracketDepth > 0) squareBracketDepth--; return RIGHT_BRACKET;
-        case '{':  afterStruct = false; afterBuffer = false; inDeclaratorList = false; afterDeclarator = false; angleBracketDepth = 0; squareBracketDepth = 0; return LEFT_BRACE;
-        case '}':  inDeclaratorList = false; afterDeclarator = false; angleBracketDepth = 0; squareBracketDepth = 0; return RIGHT_BRACE;
+        case '[':                       return LEFT_BRACKET;
+        case ']':                       return RIGHT_BRACKET;
+        case '{':  afterStruct = false; afterBuffer = false; return LEFT_BRACE;
+        case '}':                       return RIGHT_BRACE;
         case '\\':
         case '\\':
             parseContext.error(loc, "illegal use of escape character", "\\", "");
             parseContext.error(loc, "illegal use of escape character", "\\", "");
             break;
             break;
@@ -1950,26 +1941,13 @@ int TScanContext::identifierOrType()
             if (variable->isUserType() &&
             if (variable->isUserType() &&
                 // treat redeclaration of forward-declared buffer/uniform reference as an identifier
                 // treat redeclaration of forward-declared buffer/uniform reference as an identifier
                 !(variable->getType().isReference() && afterBuffer)) {
                 !(variable->getType().isReference() && afterBuffer)) {
-
-                // If we're in a declarator list (like "float a, B;"), treat struct names as IDENTIFIER
-                // to fix GitHub issue #3931
-                if (inDeclaratorList) {
-                    return IDENTIFIER;
-                }
-                
                 afterType = true;
                 afterType = true;
+
                 return TYPE_NAME;
                 return TYPE_NAME;
             }
             }
         }
         }
     }
     }
 
 
-    // If we see an identifier right after a type, this might be a declarator.
-    // But not in template parameters (inside angle brackets) or array expressions (inside square brackets)
-    if (afterType && angleBracketDepth == 0 && squareBracketDepth == 0) {
-        afterDeclarator = true;
-        afterType = false;
-    }
-
     return IDENTIFIER;
     return IDENTIFIER;
 }
 }
 
 

+ 1 - 5
glslang/MachineIndependent/ScanContext.h

@@ -53,7 +53,7 @@ public:
     explicit TScanContext(TParseContextBase& pc) :
     explicit TScanContext(TParseContextBase& pc) :
         parseContext(pc),
         parseContext(pc),
         afterType(false), afterStruct(false),
         afterType(false), afterStruct(false),
-        field(false), afterBuffer(false), inDeclaratorList(false), afterDeclarator(false), angleBracketDepth(0), squareBracketDepth(0) { }
+        field(false), afterBuffer(false) { }
     virtual ~TScanContext() { }
     virtual ~TScanContext() { }
 
 
     static void fillInKeywordMap();
     static void fillInKeywordMap();
@@ -82,10 +82,6 @@ protected:
     bool afterStruct;         // true if we've recognized the STRUCT keyword, so can only be looking for an identifier
     bool afterStruct;         // true if we've recognized the STRUCT keyword, so can only be looking for an identifier
     bool field;               // true if we're on a field, right after a '.'
     bool field;               // true if we're on a field, right after a '.'
     bool afterBuffer;         // true if we've recognized the BUFFER keyword
     bool afterBuffer;         // true if we've recognized the BUFFER keyword
-    bool inDeclaratorList;    // true if we detected we're in a declarator list like "float a, b;"
-    bool afterDeclarator;     // true if we just saw an identifier after a type (potential declarator)
-    int angleBracketDepth;    // track nesting level of < > to detect template parameters
-    int squareBracketDepth;   // track nesting level of [ ] to detect array expressions
     TSourceLoc loc;
     TSourceLoc loc;
     TParserToken* parserToken;
     TParserToken* parserToken;
     TPpToken* ppToken;
     TPpToken* ppToken;

+ 0 - 1
gtests/CMakeLists.txt

@@ -56,7 +56,6 @@ if(GLSLANG_TESTS)
             ${CMAKE_CURRENT_SOURCE_DIR}/LiveTraverser.FromFile.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/LiveTraverser.FromFile.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/Pp.FromFile.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/Pp.FromFile.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/Spv.FromFile.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/Spv.FromFile.cpp
-            ${CMAKE_CURRENT_SOURCE_DIR}/StructName.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/VkRelaxed.FromFile.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/VkRelaxed.FromFile.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/GlslMapIO.FromFile.cpp)
             ${CMAKE_CURRENT_SOURCE_DIR}/GlslMapIO.FromFile.cpp)
 
 

+ 0 - 80
gtests/StructName.cpp

@@ -1,80 +0,0 @@
-// Copyright (C) 2025 NVIDIA Corporation
-//
-// All rights reserved.
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions
-// are met:
-//
-//    Redistributions of source code must retain the above copyright
-//    notice, this list of conditions and the following disclaimer.
-//
-//    Redistributions in binary form must reproduce the above
-//    copyright notice, this list of conditions and the following
-//    disclaimer in the documentation and/or other materials provided
-//    with the distribution.
-//
-//    Neither the name of 3Dlabs Inc. Ltd. nor the names of its
-//    contributors may be used to endorse or promote products derived
-//    from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
-// FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
-// COPYRIGHT HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
-// INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
-// BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
-// LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
-// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
-// LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
-// ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
-// POSSIBILITY OF SUCH DAMAGE.
-
-#include <gtest/gtest.h>
-
-#include "TestFixture.h"
-
-namespace glslangtest {
-namespace {
-
-using StructNameTest = GlslangTest<::testing::Test>;
-
-// Test the original bug report case from issue #3931.
-TEST_F(StructNameTest, OriginalBugReportStructMember)
-{
-    const std::string inputFname = GlobalTestSettings.testRoot + "/struct_name_as_struct_member.frag";
-    std::string input;
-    tryLoadFile(inputFname, "input", &input);
-
-    EShMessages controls = DeriveOptions(Source::GLSL, Semantics::OpenGL, Target::AST);
-    GlslangResult result = compileAndLink("struct_name_as_struct_member.frag", input, "", controls,
-                                          glslang::EShTargetVulkan_1_0, glslang::EShTargetSpv_1_0);
-
-    // Should NOT have the original syntax error that was reported in the GitHub issue.
-    EXPECT_EQ(result.linkingError.find("syntax error, unexpected TYPE_NAME, expecting IDENTIFIER"), std::string::npos);
-
-    // Should compile successfully.
-    EXPECT_EQ(result.shaderResults[0].output.find("compilation errors"), std::string::npos);
-}
-
-// Test struct names used as array member names.
-TEST_F(StructNameTest, StructNameAsArrayMember)
-{
-    const std::string inputFname = GlobalTestSettings.testRoot + "/struct_name_as_array_member.frag";
-    std::string input;
-    tryLoadFile(inputFname, "input", &input);
-
-    EShMessages controls = DeriveOptions(Source::GLSL, Semantics::OpenGL, Target::AST);
-    GlslangResult result = compileAndLink("struct_name_as_array_member.frag", input, "", controls,
-                                          glslang::EShTargetVulkan_1_0, glslang::EShTargetSpv_1_0);
-
-    // Should NOT have the original syntax error that was reported in the GitHub issue.
-    EXPECT_EQ(result.linkingError.find("syntax error, unexpected TYPE_NAME, expecting IDENTIFIER"), std::string::npos);
-
-    // Should compile successfully.
-    EXPECT_EQ(result.shaderResults[0].output.find("compilation errors"), std::string::npos);
-}
-
-} // anonymous namespace
-} // namespace glslangtest