Enhance readability of error messages for GLSL

Specifically, make GLSL link error messages more specific and output
only information relevant to the error.

Also change type printing to more closely reflect GLSL syntax. This
is the default for link error messages, but must me enabled with the
new option --enhanced-msgs for compilation error messages.

Also with --enhanced-msgs, only emit one error message per source
line.
This commit is contained in:
Greg Fischer 2021-12-30 11:56:57 -07:00
parent c34bb3b6c5
commit ca0d54d51b
56 changed files with 1055 additions and 374 deletions

View file

@ -902,8 +902,10 @@ TIntermTyped* TParseContext::handleBinaryMath(const TSourceLoc& loc, const char*
result = intermediate.addBinaryMath(op, left, right, loc);
}
if (result == nullptr)
binaryOpError(loc, str, left->getCompleteString(), right->getCompleteString());
if (result == nullptr) {
bool enhanced = intermediate.getEnhancedMsgs();
binaryOpError(loc, str, left->getCompleteString(enhanced), right->getCompleteString(enhanced));
}
return result;
}
@ -926,8 +928,10 @@ TIntermTyped* TParseContext::handleUnaryMath(const TSourceLoc& loc, const char*
if (result)
return result;
else
unaryOpError(loc, str, childNode->getCompleteString());
else {
bool enhanced = intermediate.getEnhancedMsgs();
unaryOpError(loc, str, childNode->getCompleteString(enhanced));
}
return childNode;
}
@ -953,8 +957,8 @@ TIntermTyped* TParseContext::handleDotDereference(const TSourceLoc& loc, TInterm
requireProfile(loc, ~EEsProfile, feature);
profileRequires(loc, ~EEsProfile, 420, E_GL_ARB_shading_language_420pack, feature);
} else if (!base->getType().isCoopMat()) {
error(loc, "does not operate on this type:", field.c_str(), base->getType().getCompleteString().c_str());
bool enhanced = intermediate.getEnhancedMsgs();
error(loc, "does not operate on this type:", field.c_str(), base->getType().getCompleteString(enhanced).c_str());
return base;
}
@ -1005,10 +1009,16 @@ TIntermTyped* TParseContext::handleDotDereference(const TSourceLoc& loc, TInterm
intermediate.addIoAccessed(field);
}
inheritMemoryQualifiers(base->getQualifier(), result->getWritableType().getQualifier());
} else
error(loc, "no such field in structure", field.c_str(), "");
} else {
auto baseSymbol = base;
while (baseSymbol->getAsSymbolNode() == nullptr)
baseSymbol = baseSymbol->getAsBinaryNode()->getLeft();
TString structName;
structName.append("\'").append(baseSymbol->getAsSymbolNode()->getName().c_str()).append( "\'");
error(loc, "no such field in structure", field.c_str(), structName.c_str());
}
} else
error(loc, "does not apply to this type:", field.c_str(), base->getType().getCompleteString().c_str());
error(loc, "does not apply to this type:", field.c_str(), base->getType().getCompleteString(intermediate.getEnhancedMsgs()).c_str());
// Propagate noContraction up the dereference chain
if (base->getQualifier().isNoContraction())
@ -1314,7 +1324,7 @@ TIntermTyped* TParseContext::handleFunctionCall(const TSourceLoc& loc, TFunction
//
result = addConstructor(loc, arguments, type);
if (result == nullptr)
error(loc, "cannot construct with these arguments", type.getCompleteString().c_str(), "");
error(loc, "cannot construct with these arguments", type.getCompleteString(intermediate.getEnhancedMsgs()).c_str(), "");
}
} else {
//
@ -1494,7 +1504,7 @@ TIntermTyped* TParseContext::handleBuiltInFunctionCall(TSourceLoc loc, TIntermNo
else
error(arguments->getLoc(), " wrong operand type", "Internal Error",
"built in unary operator function. Type: %s",
static_cast<TIntermTyped*>(arguments)->getCompleteString().c_str());
static_cast<TIntermTyped*>(arguments)->getCompleteString(intermediate.getEnhancedMsgs()).c_str());
} else if (result->getAsOperator())
builtInOpCheck(loc, function, *result->getAsOperator());
@ -2599,23 +2609,24 @@ void TParseContext::builtInOpCheck(const TSourceLoc& loc, const TFunction& fnCan
// Check that if extended types are being used that the correct extensions are enabled.
if (arg0 != nullptr) {
const TType& type = arg0->getType();
bool enhanced = intermediate.getEnhancedMsgs();
switch (type.getBasicType()) {
default:
break;
case EbtInt8:
case EbtUint8:
requireExtensions(loc, 1, &E_GL_EXT_shader_subgroup_extended_types_int8, type.getCompleteString().c_str());
requireExtensions(loc, 1, &E_GL_EXT_shader_subgroup_extended_types_int8, type.getCompleteString(enhanced).c_str());
break;
case EbtInt16:
case EbtUint16:
requireExtensions(loc, 1, &E_GL_EXT_shader_subgroup_extended_types_int16, type.getCompleteString().c_str());
requireExtensions(loc, 1, &E_GL_EXT_shader_subgroup_extended_types_int16, type.getCompleteString(enhanced).c_str());
break;
case EbtInt64:
case EbtUint64:
requireExtensions(loc, 1, &E_GL_EXT_shader_subgroup_extended_types_int64, type.getCompleteString().c_str());
requireExtensions(loc, 1, &E_GL_EXT_shader_subgroup_extended_types_int64, type.getCompleteString(enhanced).c_str());
break;
case EbtFloat16:
requireExtensions(loc, 1, &E_GL_EXT_shader_subgroup_extended_types_float16, type.getCompleteString().c_str());
requireExtensions(loc, 1, &E_GL_EXT_shader_subgroup_extended_types_float16, type.getCompleteString(enhanced).c_str());
break;
}
}
@ -3198,6 +3209,12 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
break;
}
TString constructorString;
if (intermediate.getEnhancedMsgs())
constructorString.append(type.getCompleteString(true, false, false, true)).append(" constructor");
else
constructorString.append("constructor");
// See if it's a matrix
bool constructingMatrix = false;
switch (op) {
@ -3255,7 +3272,7 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
if (function[arg].type->isArray()) {
if (function[arg].type->isUnsizedArray()) {
// Can't construct from an unsized array.
error(loc, "array argument must be sized", "constructor", "");
error(loc, "array argument must be sized", constructorString.c_str(), "");
return true;
}
arrayArg = true;
@ -3285,13 +3302,13 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
intArgument = true;
if (type.isStruct()) {
if (function[arg].type->contains16BitFloat()) {
requireFloat16Arithmetic(loc, "constructor", "can't construct structure containing 16-bit type");
requireFloat16Arithmetic(loc, constructorString.c_str(), "can't construct structure containing 16-bit type");
}
if (function[arg].type->contains16BitInt()) {
requireInt16Arithmetic(loc, "constructor", "can't construct structure containing 16-bit type");
requireInt16Arithmetic(loc, constructorString.c_str(), "can't construct structure containing 16-bit type");
}
if (function[arg].type->contains8BitInt()) {
requireInt8Arithmetic(loc, "constructor", "can't construct structure containing 8-bit type");
requireInt8Arithmetic(loc, constructorString.c_str(), "can't construct structure containing 8-bit type");
}
}
}
@ -3305,9 +3322,9 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
case EOpConstructF16Vec3:
case EOpConstructF16Vec4:
if (type.isArray())
requireFloat16Arithmetic(loc, "constructor", "16-bit arrays not supported");
requireFloat16Arithmetic(loc, constructorString.c_str(), "16-bit arrays not supported");
if (type.isVector() && function.getParamCount() != 1)
requireFloat16Arithmetic(loc, "constructor", "16-bit vectors only take vector types");
requireFloat16Arithmetic(loc, constructorString.c_str(), "16-bit vectors only take vector types");
break;
case EOpConstructUint16:
case EOpConstructU16Vec2:
@ -3318,9 +3335,9 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
case EOpConstructI16Vec3:
case EOpConstructI16Vec4:
if (type.isArray())
requireInt16Arithmetic(loc, "constructor", "16-bit arrays not supported");
requireInt16Arithmetic(loc, constructorString.c_str(), "16-bit arrays not supported");
if (type.isVector() && function.getParamCount() != 1)
requireInt16Arithmetic(loc, "constructor", "16-bit vectors only take vector types");
requireInt16Arithmetic(loc, constructorString.c_str(), "16-bit vectors only take vector types");
break;
case EOpConstructUint8:
case EOpConstructU8Vec2:
@ -3331,9 +3348,9 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
case EOpConstructI8Vec3:
case EOpConstructI8Vec4:
if (type.isArray())
requireInt8Arithmetic(loc, "constructor", "8-bit arrays not supported");
requireInt8Arithmetic(loc, constructorString.c_str(), "8-bit arrays not supported");
if (type.isVector() && function.getParamCount() != 1)
requireInt8Arithmetic(loc, "constructor", "8-bit vectors only take vector types");
requireInt8Arithmetic(loc, constructorString.c_str(), "8-bit vectors only take vector types");
break;
default:
break;
@ -3415,7 +3432,7 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
if (type.isArray()) {
if (function.getParamCount() == 0) {
error(loc, "array constructor must have at least one argument", "constructor", "");
error(loc, "array constructor must have at least one argument", constructorString.c_str(), "");
return true;
}
@ -3423,7 +3440,7 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
// auto adapt the constructor type to the number of arguments
type.changeOuterArraySize(function.getParamCount());
} else if (type.getOuterArraySize() != function.getParamCount()) {
error(loc, "array constructor needs one argument per array element", "constructor", "");
error(loc, "array constructor needs one argument per array element", constructorString.c_str(), "");
return true;
}
@ -3436,7 +3453,7 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
// At least the dimensionalities have to match.
if (! function[0].type->isArray() ||
arraySizes.getNumDims() != function[0].type->getArraySizes()->getNumDims() + 1) {
error(loc, "array constructor argument not correct type to construct array element", "constructor", "");
error(loc, "array constructor argument not correct type to construct array element", constructorString.c_str(), "");
return true;
}
@ -3453,7 +3470,7 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
}
if (arrayArg && op != EOpConstructStruct && ! type.isArrayOfArrays()) {
error(loc, "constructing non-array constituent from array argument", "constructor", "");
error(loc, "constructing non-array constituent from array argument", constructorString.c_str(), "");
return true;
}
@ -3463,51 +3480,51 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
// "If a matrix argument is given to a matrix constructor,
// it is a compile-time error to have any other arguments."
if (function.getParamCount() != 1)
error(loc, "matrix constructed from matrix can only have one argument", "constructor", "");
error(loc, "matrix constructed from matrix can only have one argument", constructorString.c_str(), "");
return false;
}
if (overFull) {
error(loc, "too many arguments", "constructor", "");
error(loc, "too many arguments", constructorString.c_str(), "");
return true;
}
if (op == EOpConstructStruct && ! type.isArray() && (int)type.getStruct()->size() != function.getParamCount()) {
error(loc, "Number of constructor parameters does not match the number of structure fields", "constructor", "");
error(loc, "Number of constructor parameters does not match the number of structure fields", constructorString.c_str(), "");
return true;
}
if ((op != EOpConstructStruct && size != 1 && size < type.computeNumComponents()) ||
(op == EOpConstructStruct && size < type.computeNumComponents())) {
error(loc, "not enough data provided for construction", "constructor", "");
error(loc, "not enough data provided for construction", constructorString.c_str(), "");
return true;
}
if (type.isCoopMat() && function.getParamCount() != 1) {
error(loc, "wrong number of arguments", "constructor", "");
error(loc, "wrong number of arguments", constructorString.c_str(), "");
return true;
}
if (type.isCoopMat() &&
!(function[0].type->isScalar() || function[0].type->isCoopMat())) {
error(loc, "Cooperative matrix constructor argument must be scalar or cooperative matrix", "constructor", "");
error(loc, "Cooperative matrix constructor argument must be scalar or cooperative matrix", constructorString.c_str(), "");
return true;
}
TIntermTyped* typed = node->getAsTyped();
if (typed == nullptr) {
error(loc, "constructor argument does not have a type", "constructor", "");
error(loc, "constructor argument does not have a type", constructorString.c_str(), "");
return true;
}
if (op != EOpConstructStruct && op != EOpConstructNonuniform && typed->getBasicType() == EbtSampler) {
error(loc, "cannot convert a sampler", "constructor", "");
error(loc, "cannot convert a sampler", constructorString.c_str(), "");
return true;
}
if (op != EOpConstructStruct && typed->isAtomic()) {
error(loc, "cannot convert an atomic_uint", "constructor", "");
error(loc, "cannot convert an atomic_uint", constructorString.c_str(), "");
return true;
}
if (typed->getBasicType() == EbtVoid) {
error(loc, "cannot convert a void", "constructor", "");
error(loc, "cannot convert a void", constructorString.c_str(), "");
return true;
}
@ -7430,14 +7447,14 @@ TIntermNode* TParseContext::executeInitializer(const TSourceLoc& loc, TIntermTyp
// Uniforms require a compile-time constant initializer
if (qualifier == EvqUniform && ! initializer->getType().getQualifier().isFrontEndConstant()) {
error(loc, "uniform initializers must be constant", "=", "'%s'",
variable->getType().getCompleteString().c_str());
variable->getType().getCompleteString(intermediate.getEnhancedMsgs()).c_str());
variable->getWritableType().getQualifier().makeTemporary();
return nullptr;
}
// Global consts require a constant initializer (specialization constant is okay)
if (qualifier == EvqConst && symbolTable.atGlobalLevel() && ! initializer->getType().getQualifier().isConstant()) {
error(loc, "global const initializers must be constant", "=", "'%s'",
variable->getType().getCompleteString().c_str());
variable->getType().getCompleteString(intermediate.getEnhancedMsgs()).c_str());
variable->getWritableType().getQualifier().makeTemporary();
return nullptr;
}
@ -7500,7 +7517,7 @@ TIntermNode* TParseContext::executeInitializer(const TSourceLoc& loc, TIntermTyp
TIntermSymbol* intermSymbol = intermediate.addSymbol(*variable, loc);
TIntermTyped* initNode = intermediate.addAssign(EOpAssign, intermSymbol, initializer, loc);
if (! initNode)
assignError(loc, "=", intermSymbol->getCompleteString(), initializer->getCompleteString());
assignError(loc, "=", intermSymbol->getCompleteString(intermediate.getEnhancedMsgs()), initializer->getCompleteString(intermediate.getEnhancedMsgs()));
return initNode;
}
@ -7571,7 +7588,7 @@ TIntermTyped* TParseContext::convertInitializerList(const TSourceLoc& loc, const
}
} else if (type.isMatrix()) {
if (type.getMatrixCols() != (int)initList->getSequence().size()) {
error(loc, "wrong number of matrix columns:", "initializer list", type.getCompleteString().c_str());
error(loc, "wrong number of matrix columns:", "initializer list", type.getCompleteString(intermediate.getEnhancedMsgs()).c_str());
return nullptr;
}
TType vectorType(type, 0); // dereferenced type
@ -7582,20 +7599,20 @@ TIntermTyped* TParseContext::convertInitializerList(const TSourceLoc& loc, const
}
} else if (type.isVector()) {
if (type.getVectorSize() != (int)initList->getSequence().size()) {
error(loc, "wrong vector size (or rows in a matrix column):", "initializer list", type.getCompleteString().c_str());
error(loc, "wrong vector size (or rows in a matrix column):", "initializer list", type.getCompleteString(intermediate.getEnhancedMsgs()).c_str());
return nullptr;
}
TBasicType destType = type.getBasicType();
for (int i = 0; i < type.getVectorSize(); ++i) {
TBasicType initType = initList->getSequence()[i]->getAsTyped()->getBasicType();
if (destType != initType && !intermediate.canImplicitlyPromote(initType, destType)) {
error(loc, "type mismatch in initializer list", "initializer list", type.getCompleteString().c_str());
error(loc, "type mismatch in initializer list", "initializer list", type.getCompleteString(intermediate.getEnhancedMsgs()).c_str());
return nullptr;
}
}
} else {
error(loc, "unexpected initializer-list type:", "initializer list", type.getCompleteString().c_str());
error(loc, "unexpected initializer-list type:", "initializer list", type.getCompleteString(intermediate.getEnhancedMsgs()).c_str());
return nullptr;
}
@ -8103,8 +8120,9 @@ TIntermTyped* TParseContext::constructAggregate(TIntermNode* node, const TType&
{
TIntermTyped* converted = intermediate.addConversion(EOpConstructStruct, type, node->getAsTyped());
if (! converted || converted->getType() != type) {
bool enhanced = intermediate.getEnhancedMsgs();
error(loc, "", "constructor", "cannot convert parameter %d from '%s' to '%s'", paramCount,
node->getAsTyped()->getType().getCompleteString().c_str(), type.getCompleteString().c_str());
node->getAsTyped()->getType().getCompleteString(enhanced).c_str(), type.getCompleteString(enhanced).c_str());
return nullptr;
}