Merge pull request #15 from google/preprocessing-error

StandAlone/StandAlone.cpp

@@ -611,6 +611,8 @@ void SetMessageOptions(EShMessages& messages)
         messages = (EShMessages)(messages | EShMsgSpvRules);
     if (Options & EOptionVulkanRules)
         messages = (EShMessages)(messages | EShMsgVulkanRules);
+    if (Options & EOptionOutputPreprocessed)
+        messages = (EShMessages)(messages | EShMsgOnlyPreprocessor);
 }
 
 //
@@ -645,13 +647,22 @@ const char* GlslStd450DebugNames[GLSL_STD_450::Count];
 
 // Outputs the given string, but only if it is non-null and non-empty.
 // This prevents erroneous newlines from appearing.
-void puts_if_non_empty(const char* str)
+void PutsIfNonEmpty(const char* str)
 {
     if (str && str[0]) {
         puts(str);
     }
 }
 
+// Outputs the given string to stderr, but only if it is non-null and non-empty.
+// This prevents erroneous newlines from appearing.
+void StderrIfNonEmpty(const char* str)
+{
+    if (str && str[0]) {
+      fprintf(stderr, "%s\n", str);
+    }
+}
+
 //
 // For linking mode: Will independently parse each item in the worklist, but then put them
 // in the same program and link them together.
@@ -689,8 +700,14 @@ void CompileAndLinkShaders()
         shader->setStrings(shaderStrings, 1);
         if (Options & EOptionOutputPreprocessed) {
             std::string str;
-            shader->preprocess(&Resources, defaultVersion, ENoProfile, false, false, messages, &str);
-            puts(str.c_str());
+            if (shader->preprocess(&Resources, defaultVersion, ENoProfile,
+                                   false, false, messages, &str)) {
+                PutsIfNonEmpty(str.c_str());
+            } else {
+                CompileFailed = true;
+            }
+            StderrIfNonEmpty(shader->getInfoLog());
+            StderrIfNonEmpty(shader->getInfoDebugLog());
             FreeFileData(shaderStrings);
             continue;
         }
@@ -700,9 +717,9 @@ void CompileAndLinkShaders()
         program.addShader(shader);
 
         if (! (Options & EOptionSuppressInfolog)) {
-            puts_if_non_empty(workItem->name.c_str());
-            puts_if_non_empty(shader->getInfoLog());
-            puts_if_non_empty(shader->getInfoDebugLog());
+            PutsIfNonEmpty(workItem->name.c_str());
+            PutsIfNonEmpty(shader->getInfoLog());
+            PutsIfNonEmpty(shader->getInfoDebugLog());
         }
 
         FreeFileData(shaderStrings);
@@ -716,8 +733,8 @@ void CompileAndLinkShaders()
         LinkFailed = true;
 
     if (! (Options & EOptionSuppressInfolog)) {
-        puts_if_non_empty(program.getInfoLog());
-        puts_if_non_empty(program.getInfoDebugLog());
+        PutsIfNonEmpty(program.getInfoLog());
+        PutsIfNonEmpty(program.getInfoDebugLog());
     }
 
     if (Options & EOptionDumpReflection) {
@@ -814,8 +831,8 @@ int C_DECL main(int argc, char* argv[])
         for (int w = 0; w < NumWorkItems; ++w) {
             if (Work[w]) {
                 if (printShaderNames)
-                    puts_if_non_empty(Work[w]->name.c_str());
-                puts_if_non_empty(Work[w]->results.c_str());
+                    PutsIfNonEmpty(Work[w]->name.c_str());
+                PutsIfNonEmpty(Work[w]->results.c_str());
                 delete Work[w];
             }
         }
@@ -943,7 +960,8 @@ void usage()
            "  -G          create SPIR-V binary, under OpenGL semantics; turns on -l;\n"
            "              default file name is <stage>.spv (-o overrides this)\n"
            "  -H          print human readable form of SPIR-V; turns on -V\n"
-           "  -E          print pre-processed GLSL; cannot be used with -l.\n"
+           "  -E          print pre-processed GLSL; cannot be used with -l;\n"
+           "              errors will appear on stderr.\n"
            "  -c          configuration dump;\n"
            "              creates the default configuration file (redirect to a .conf file)\n"
            "  -d          default to desktop (#version 110) when there is no shader #version\n"

Test/baseResults/preprocessor.edge_cases.vert.err

+Warning, version 310 is not yet complete; most version-specific features are present, but some are missing.
+

Test/baseResults/preprocessor.errors.vert.err

+Warning, version 310 is not yet complete; most version-specific features are present, but some are missing.
+ERROR: 0:9: '#error' : This should show up in pp output .
+ERROR: 0:14: '#' : invalid directive: def
+ERROR: 0:15: 'preprocessor evaluation' : undefined macro in expression not allowed in es profile Y
+ERROR: 0:21: '' : missing #endif
+ERROR: 4 compilation errors. No code generated.
+
+

Test/baseResults/preprocessor.errors.vert.out

-#version 310 es
-
-
-
-
-
-
-
-#error This should show up in pp output . 
-
-
-
-
-int main(){
-}
-

Test/baseResults/preprocessor.extensions.vert.err

+Warning, version 310 is not yet complete; most version-specific features are present, but some are missing.
+WARNING: 0:5: '#extension' : extension is only partially supported: GL_EXT_gpu_shader5
+WARNING: 0:6: '#extension' : extension not supported: GL_EXT_shader_texture_image_samples
+

Test/baseResults/preprocessor.function_macro.vert.err

+Warning, version 310 is not yet complete; most version-specific features are present, but some are missing.
+

Test/baseResults/preprocessor.pragma.vert.err

+Warning, version 310 is not yet complete; most version-specific features are present, but some are missing.
+

Test/baseResults/preprocessor.simple.vert.err

+Warning, version 310 is not yet complete; most version-specific features are present, but some are missing.
+

Test/baseResults/preprocessor.success_if_parse_would_fail.vert.out

+int x(){
+  something that shouldnt compile;
+}
+

Test/preprocessor.errors.vert

 #version 310 es
 
-#define X
+#define X 1
 
 #if X
-  #if Y
+  #ifdef Y
     #error This should not show up in pp output.
   #endif
     #error This should show up in pp output.
@@ -11,5 +11,10 @@
   #error This should not show up in pp output.
 #endif
 
+#def X
+#if Y
+
+#extension a
+
 int main() {
 }

Test/preprocessor.success_if_parse_would_fail.vert

+int x() {
+  something that shouldnt compile;
+}
+

Test/runtests

@@ -61,8 +61,9 @@ rm -f comp.spv frag.spv geom.spv tesc.spv tese.spv vert.spv
 while read t; do
     echo Running Preprocessor $t...
     b=`basename $t`
-    $EXE -E $t > $TARGETDIR/$b.out
+    $EXE -E $t > $TARGETDIR/$b.out 2> $TARGETDIR/$b.err
     diff -b $BASEDIR/$b.out $TARGETDIR/$b.out || HASERROR=1
+    diff -b $BASEDIR/$b.err $TARGETDIR/$b.err || HASERROR=1
 done < test-preprocessor-list
 
 #

Test/test-preprocessor-list

@@ -5,3 +5,4 @@ preprocessor.function_macro.vert
 preprocessor.line.vert
 preprocessor.pragma.vert
 preprocessor.simple.vert
+preprocessor.success_if_parse_would_fail.vert

glslang/MachineIndependent/ParseHelper.cpp

@@ -341,24 +341,35 @@ bool TParseContext::parseVectorFields(TSourceLoc loc, const TString& compString,
 //
 // Used to output syntax, parsing, and semantic errors.
 //
-void C_DECL TParseContext::error(TSourceLoc loc, const char* szReason, const char* szToken,
-                                 const char* szExtraInfoFormat, ...)
+
+void TParseContext::outputMessage(TSourceLoc loc, const char* szReason,
+                                  const char* szToken,
+                                  const char* szExtraInfoFormat,
+                                  TPrefixType prefix, va_list args)
 {
     const int maxSize = GlslangMaxTokenLength + 200;
     char szExtraInfo[maxSize];
-    va_list marker;
-
-    va_start(marker, szExtraInfoFormat);
 
-    safe_vsprintf(szExtraInfo, maxSize, szExtraInfoFormat, marker);
+    safe_vsprintf(szExtraInfo, maxSize, szExtraInfoFormat, args);
 
-    infoSink.info.prefix(EPrefixError);
+    infoSink.info.prefix(prefix);
     infoSink.info.location(loc);
     infoSink.info << "'" << szToken <<  "' : " << szReason << " " << szExtraInfo << "\n";
 
-    va_end(marker);
-
+    if (prefix == EPrefixError) {
         ++numErrors;
+    }
+}
+
+void C_DECL TParseContext::error(TSourceLoc loc, const char* szReason, const char* szToken,
+                                 const char* szExtraInfoFormat, ...)
+{
+    if (messages & EShMsgOnlyPreprocessor)
+      return;
+    va_list args;
+    va_start(args, szExtraInfoFormat);
+    outputMessage(loc, szReason, szToken, szExtraInfoFormat, EPrefixError, args);
+    va_end(args);
 }
 
 void C_DECL TParseContext::warn(TSourceLoc loc, const char* szReason, const char* szToken,
@@ -366,20 +377,28 @@ void C_DECL TParseContext::warn(TSourceLoc loc, const char* szReason, const char
 {
     if (suppressWarnings())
         return;
+    va_list args;
+    va_start(args, szExtraInfoFormat);
+    outputMessage(loc, szReason, szToken, szExtraInfoFormat, EPrefixWarning, args);
+    va_end(args);
+}
 
-    const int maxSize = GlslangMaxTokenLength + 200;
-    char szExtraInfo[maxSize];
-    va_list marker;
-
-    va_start(marker, szExtraInfoFormat);
-
-    safe_vsprintf(szExtraInfo, maxSize, szExtraInfoFormat, marker);
-
-    infoSink.info.prefix(EPrefixWarning);
-    infoSink.info.location(loc);
-    infoSink.info << "'" << szToken <<  "' : " << szReason << " " << szExtraInfo << "\n";
+void C_DECL TParseContext::ppError(TSourceLoc loc, const char* szReason, const char* szToken,
+                                 const char* szExtraInfoFormat, ...)
+{
+    va_list args;
+    va_start(args, szExtraInfoFormat);
+    outputMessage(loc, szReason, szToken, szExtraInfoFormat, EPrefixError, args);
+    va_end(args);
+}
 
-    va_end(marker);
+void C_DECL TParseContext::ppWarn(TSourceLoc loc, const char* szReason, const char* szToken,
+                                 const char* szExtraInfoFormat, ...)
+{
+    va_list args;
+    va_start(args, szExtraInfoFormat);
+    outputMessage(loc, szReason, szToken, szExtraInfoFormat, EPrefixWarning, args);
+    va_end(args);
 }
 
 //
@@ -1815,18 +1834,18 @@ void TParseContext::reservedPpErrorCheck(TSourceLoc loc, const char* identifier,
     // compile-time error."
     // however, before that, ES tests required an error.
     if (strncmp(identifier, "GL_", 3) == 0)
-        error(loc, "names beginning with \"GL_\" can't be (un)defined:", op,  identifier);
+        ppError(loc, "names beginning with \"GL_\" can't be (un)defined:", op,  identifier);
     else if (strstr(identifier, "__") != 0) {
         if (profile == EEsProfile && version >= 300 &&
             (strcmp(identifier, "__LINE__") == 0 ||
              strcmp(identifier, "__FILE__") == 0 ||
              strcmp(identifier, "__VERSION__") == 0))
-            error(loc, "predefined names can't be (un)defined:", op,  identifier);
+            ppError(loc, "predefined names can't be (un)defined:", op,  identifier);
         else {
             if (profile == EEsProfile && version <= 300)
-                error(loc, "names containing consecutive underscores are reserved, and an error if version <= 300:", op, identifier);
+                ppError(loc, "names containing consecutive underscores are reserved, and an error if version <= 300:", op, identifier);
             else
-                warn(loc, "names containing consecutive underscores are reserved:", op, identifier);
+                ppWarn(loc, "names containing consecutive underscores are reserved:", op, identifier);
         }
     }
 }

glslang/MachineIndependent/ParseHelper.h

@@ -78,6 +78,10 @@ public:
                       const char* szExtraInfoFormat, ...);
     void C_DECL  warn(TSourceLoc, const char* szReason, const char* szToken,
                       const char* szExtraInfoFormat, ...);
+    void C_DECL ppError(TSourceLoc, const char* szReason, const char* szToken,
+                      const char* szExtraInfoFormat, ...);
+    void C_DECL ppWarn(TSourceLoc, const char* szReason, const char* szToken,
+                      const char* szExtraInfoFormat, ...);
 
     bool relaxedErrors()    const { return (messages & EShMsgRelaxedErrors)    != 0; }
     bool suppressWarnings() const { return (messages & EShMsgSuppressWarnings) != 0; }
@@ -250,6 +254,9 @@ protected:
     TOperator mapTypeToConstructorOp(const TType&) const;
     void updateExtensionBehavior(const char* const extension, TExtensionBehavior);
     void finalErrorCheck();
+    void outputMessage(TSourceLoc, const char* szReason, const char* szToken,
+                       const char* szExtraInfoFormat, TPrefixType prefix,
+                       va_list args);
 
 public:
     //

glslang/MachineIndependent/ShaderLang.cpp

@@ -714,7 +714,13 @@ struct DoPreprocessing {
         outputStream << std::endl;
         *outputString = outputStream.str();
 
-        return true;
+        bool success = true;
+        if (parseContext.getNumErrors() > 0) {
+            success = false;
+            parseContext.infoSink.info.prefix(EPrefixError);
+            parseContext.infoSink.info << parseContext.getNumErrors() << " compilation errors.  No code generated.\n\n";
+        }
+        return success;
     }
     std::string* outputString;
 };

glslang/Public/ShaderLang.h

@@ -131,6 +131,7 @@ enum EShMessages {
     EShMsgAST              = (1 << 2),  // print the AST intermediate representation
     EShMsgSpvRules         = (1 << 3),  // issue messages for SPIR-V generation
     EShMsgVulkanRules      = (1 << 4),  // issue messages for Vulkan-requirements of GLSL for SPIR-V
+    EShMsgOnlyPreprocessor = (1 << 5),  // only print out errors produced by the preprocessor
 };
 
 //