From dc9f6f61adaec755a09e1943cf7014c688443bcb Mon Sep 17 00:00:00 2001 From: Antoine Date: Sat, 20 Jul 2024 00:37:58 +0200 Subject: [PATCH] Add column to location logs This option can be enabled using the new --error-column option to the command line utility. It can also be enabled programatically. --- StandAlone/StandAlone.cpp | 6 +++++ Test/baseResults/error-column.vert.out | 12 +++++++++ Test/error-column.vert | 27 +++++++++++++++++++ Test/runtests | 7 +++++ glslang/CInterface/glslang_c_interface.cpp | 2 ++ glslang/Include/InfoSink.h | 14 +++++++--- glslang/Include/glslang_c_shader_types.h | 1 + .../MachineIndependent/ParseContextBase.cpp | 2 +- glslang/MachineIndependent/Versions.cpp | 12 ++++++--- glslang/Public/ShaderLang.h | 1 + gtests/TestFixture.h | 4 +-- 11 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 Test/baseResults/error-column.vert.out create mode 100644 Test/error-column.vert diff --git a/StandAlone/StandAlone.cpp b/StandAlone/StandAlone.cpp index ac967f2b5..94851ba35 100644 --- a/StandAlone/StandAlone.cpp +++ b/StandAlone/StandAlone.cpp @@ -110,6 +110,7 @@ enum TOptions : uint64_t { EOptionInvertY = (1ull << 30), EOptionDumpBareVersion = (1ull << 31), EOptionCompileOnly = (1ull << 32), + EOptionDisplayErrorColumn = (1ull << 33), }; bool targetHlslFunctionality1 = false; bool SpvToolsDisassembler = false; @@ -898,6 +899,8 @@ void ProcessArguments(std::vector>& workItem Options |= EOptionDumpVersions; } else if (lowerword == "no-link") { Options |= EOptionCompileOnly; + } else if (lowerword == "error-column") { + Options |= EOptionDisplayErrorColumn; } else if (lowerword == "help") { usage(); break; @@ -1164,6 +1167,8 @@ void SetMessageOptions(EShMessages& messages) messages = (EShMessages)(messages | EShMsgEnhanced); if (AbsolutePath) messages = (EShMessages)(messages | EShMsgAbsolutePath); + if (Options & EOptionDisplayErrorColumn) + messages = (EShMessages)(messages | EShMsgDisplayErrorColumn); } // @@ -2024,6 +2029,7 @@ void usage() " shaders compatible with DirectX\n" " --invert-y | --iy invert position.Y output in vertex shader\n" " --enhanced-msgs print more readable error messages (GLSL only)\n" + " --error-column display the column of the error along the line\n" " --keep-uncalled | --ku don't eliminate uncalled functions\n" " --nan-clamp favor non-NaN operand in min, max, and clamp\n" " --no-storage-format | --nsf use Unknown image format\n" diff --git a/Test/baseResults/error-column.vert.out b/Test/baseResults/error-column.vert.out new file mode 100644 index 000000000..6538e82a2 --- /dev/null +++ b/Test/baseResults/error-column.vert.out @@ -0,0 +1,12 @@ +error-column.vert +ERROR: error-column.vert:7:16: 'model' : member storage qualifier cannot contradict block storage qualifier +ERROR: error-column.vert:8:16: 'view' : member storage qualifier cannot contradict block storage qualifier +ERROR: error-column.vert:9:16: 'projection' : member storage qualifier cannot contradict block storage qualifier +ERROR: error-column.vert:17:16: 'texCoord1' : member storage qualifier cannot contradict block storage qualifier +ERROR: error-column.vert:24:52: 'Pos' : undeclared identifier +ERROR: error-column.vert:24:60: 'constructor' : not enough data provided for construction +ERROR: error-column.vert:24:17: 'assign' : cannot convert from ' temp highp 4X4 matrix of float' to ' gl_Position 4-component vector of float Position' +ERROR: 7 compilation errors. No code generated. + + +SPIR-V is not generated for failed compile or link diff --git a/Test/error-column.vert b/Test/error-column.vert new file mode 100644 index 000000000..2ed97e7e0 --- /dev/null +++ b/Test/error-column.vert @@ -0,0 +1,27 @@ +#version 450 core + +layout (location = 0) in vec3 aPos; +layout (location = 1) in vec2 aTexCoords; + +layout (binding = 0) uniform block { + const mat4 model; + const mat4 view; + const mat4 projection; +}; + +layout (location = 0) out Vertex +{ + vec3 color; + vec3 worldSpacePos; + vec3 worldSpaceNorm; + const vec2 texCoord1; + flat int cameraIndex; + float ii; +} vs_out; + +void main() +{ + gl_Position = projection * view * model * vec4(Pos, 1.0); + vs_out.texCoord1 = aTexCoords; +} + diff --git a/Test/runtests b/Test/runtests index 00c2babda..6b23cd441 100755 --- a/Test/runtests +++ b/Test/runtests @@ -348,6 +348,13 @@ diff -b $BASEDIR/enhanced.7.link.out "$TARGETDIR/enhanced.7.link.out" || HASERRO run --enhanced-msgs -V --target-env vulkan1.2 --amb --aml spv.textureError.frag > "$TARGETDIR/spv.textureError.frag.out" diff -b $BASEDIR/spv.textureError.frag.out "$TARGETDIR/spv.textureError.frag.out" || HASERROR=1 +# +# Test error column +# +echo "Testing error-column" +run --error-column -C -V error-column.vert > "$TARGETDIR/error-column.vert.out" +diff -b $BASEDIR/error-column.vert.out $TARGETDIR/error-column.vert.out || HASERROR=1 + # # Test UTF8BOM # diff --git a/glslang/CInterface/glslang_c_interface.cpp b/glslang/CInterface/glslang_c_interface.cpp index cea965d40..f8fd889b7 100644 --- a/glslang/CInterface/glslang_c_interface.cpp +++ b/glslang/CInterface/glslang_c_interface.cpp @@ -205,7 +205,9 @@ static int c_shader_messages(glslang_messages_t messages) CONVERT_MSG(GLSLANG_MSG_HLSL_LEGALIZATION_BIT, EShMsgHlslLegalization); CONVERT_MSG(GLSLANG_MSG_HLSL_DX9_COMPATIBLE_BIT, EShMsgHlslDX9Compatible); CONVERT_MSG(GLSLANG_MSG_BUILTIN_SYMBOL_TABLE_BIT, EShMsgBuiltinSymbolTable); + CONVERT_MSG(GLSLANG_MSG_ENHANCED, EShMsgEnhanced); CONVERT_MSG(GLSLANG_MSG_ABSOLUTE_PATH, EShMsgAbsolutePath); + CONVERT_MSG(GLSLANG_MSG_DISPLAY_ERROR_COLUMN, EShMsgDisplayErrorColumn); return res; #undef CONVERT_MSG } diff --git a/glslang/Include/InfoSink.h b/glslang/Include/InfoSink.h index 23f495dcb..262933941 100644 --- a/glslang/Include/InfoSink.h +++ b/glslang/Include/InfoSink.h @@ -95,10 +95,14 @@ public: default: append("UNKNOWN ERROR: "); break; } } - void location(const TSourceLoc& loc, bool absolute = false) { + void location(const TSourceLoc& loc, bool absolute = false, bool displayColumn = false) { const int maxSize = 24; char locText[maxSize]; - snprintf(locText, maxSize, ":%d", loc.line); + if (displayColumn) { + snprintf(locText, maxSize, ":%d:%d", loc.line, loc.column); + } else { + snprintf(locText, maxSize, ":%d", loc.line); + } if(loc.getFilename() == nullptr && shaderFileName != nullptr && absolute) { append(std::filesystem::absolute(shaderFileName).string()); @@ -119,9 +123,11 @@ public: append(s); append("\n"); } - void message(TPrefixType message, const char* s, const TSourceLoc& loc) { + void message(TPrefixType message, const char* s, const TSourceLoc& loc, bool absolute = false, + bool displayColumn = false) + { prefix(message); - location(loc); + location(loc, absolute, displayColumn); append(s); append("\n"); } diff --git a/glslang/Include/glslang_c_shader_types.h b/glslang/Include/glslang_c_shader_types.h index 51f5642ab..7bb0ccda2 100644 --- a/glslang/Include/glslang_c_shader_types.h +++ b/glslang/Include/glslang_c_shader_types.h @@ -175,6 +175,7 @@ typedef enum { GLSLANG_MSG_BUILTIN_SYMBOL_TABLE_BIT = (1 << 14), GLSLANG_MSG_ENHANCED = (1 << 15), GLSLANG_MSG_ABSOLUTE_PATH = (1 << 16), + GLSLANG_MSG_DISPLAY_ERROR_COLUMN = (1 << 17), LAST_ELEMENT_MARKER(GLSLANG_MSG_COUNT), } glslang_messages_t; diff --git a/glslang/MachineIndependent/ParseContextBase.cpp b/glslang/MachineIndependent/ParseContextBase.cpp index 591dfc718..f7895d979 100644 --- a/glslang/MachineIndependent/ParseContextBase.cpp +++ b/glslang/MachineIndependent/ParseContextBase.cpp @@ -59,7 +59,7 @@ void TParseContextBase::outputMessage(const TSourceLoc& loc, const char* szReaso safe_vsprintf(szExtraInfo, maxSize, szExtraInfoFormat, args); infoSink.info.prefix(prefix); - infoSink.info.location(loc, messages & EShMsgAbsolutePath); + infoSink.info.location(loc, messages & EShMsgAbsolutePath, messages & EShMsgDisplayErrorColumn); infoSink.info << "'" << szToken << "' : " << szReason << " " << szExtraInfo << "\n"; if (prefix == EPrefixError) { diff --git a/glslang/MachineIndependent/Versions.cpp b/glslang/MachineIndependent/Versions.cpp index e016ef6b9..0262c54dc 100644 --- a/glslang/MachineIndependent/Versions.cpp +++ b/glslang/MachineIndependent/Versions.cpp @@ -775,7 +775,7 @@ void TParseVersions::profileRequires(const TSourceLoc& loc, int profileMask, int for (int i = 0; i < numExtensions; ++i) { switch (getExtensionBehavior(extensions[i])) { case EBhWarn: - infoSink.info.message(EPrefixWarning, ("extension " + TString(extensions[i]) + " is being used for " + featureDesc).c_str(), loc); + infoSink.info.message(EPrefixWarning, ("extension " + TString(extensions[i]) + " is being used for " + featureDesc).c_str(), loc, messages & EShMsgAbsolutePath, messages & EShMsgDisplayErrorColumn); [[fallthrough]]; case EBhRequire: case EBhEnable: @@ -813,7 +813,8 @@ void TParseVersions::checkDeprecated(const TSourceLoc& loc, int profileMask, int error(loc, "deprecated, may be removed in future release", featureDesc, ""); else if (! suppressWarnings()) infoSink.info.message(EPrefixWarning, (TString(featureDesc) + " deprecated in version " + - String(depVersion) + "; may be removed in future release").c_str(), loc); + String(depVersion) + "; may be removed in future release").c_str(), + loc, messages & EShMsgAbsolutePath, messages & EShMsgDisplayErrorColumn); } } } @@ -850,11 +851,14 @@ bool TParseVersions::checkExtensionsRequested(const TSourceLoc& loc, int numExte for (int i = 0; i < numExtensions; ++i) { TExtensionBehavior behavior = getExtensionBehavior(extensions[i]); if (behavior == EBhDisable && relaxedErrors()) { - infoSink.info.message(EPrefixWarning, "The following extension must be enabled to use this feature:", loc); + infoSink.info.message(EPrefixWarning, "The following extension must be enabled to use this feature:", loc, + messages & EShMsgAbsolutePath, messages & EShMsgDisplayErrorColumn); behavior = EBhWarn; } if (behavior == EBhWarn) { - infoSink.info.message(EPrefixWarning, ("extension " + TString(extensions[i]) + " is being used for " + featureDesc).c_str(), loc); + infoSink.info.message(EPrefixWarning, + ("extension " + TString(extensions[i]) + " is being used for " + featureDesc).c_str(), + loc, messages & EShMsgAbsolutePath, messages & EShMsgDisplayErrorColumn); warned = true; } } diff --git a/glslang/Public/ShaderLang.h b/glslang/Public/ShaderLang.h index b71b147a5..eac81b147 100644 --- a/glslang/Public/ShaderLang.h +++ b/glslang/Public/ShaderLang.h @@ -270,6 +270,7 @@ enum EShMessages : unsigned { EShMsgBuiltinSymbolTable = (1 << 14), // print the builtin symbol table EShMsgEnhanced = (1 << 15), // enhanced message readability EShMsgAbsolutePath = (1 << 16), // Output Absolute path for messages + EShMsgDisplayErrorColumn = (1 << 17), // Display error message column aswell as line LAST_ELEMENT_MARKER(EShMsgCount), }; diff --git a/gtests/TestFixture.h b/gtests/TestFixture.h index 315fdaec5..9777c2757 100644 --- a/gtests/TestFixture.h +++ b/gtests/TestFixture.h @@ -691,8 +691,8 @@ public: std::string ppShader; glslang::TShader::ForbidIncluder includer; const bool success = shader.preprocess( - GetDefaultResources(), defaultVersion, defaultProfile, - forceVersionProfile, isForwardCompatible, (EShMessages)(EShMsgOnlyPreprocessor | EShMsgCascadingErrors), + GetDefaultResources(), defaultVersion, defaultProfile, forceVersionProfile, isForwardCompatible, + (EShMessages)(EShMsgOnlyPreprocessor | EShMsgCascadingErrors), &ppShader, includer); std::string log = shader.getInfoLog();