Skip to content

Commit 199cb20

Browse files
authored
Improve OpSpecConstantOp failure message (KhronosGroup#6731)
Fixes KhronosGroup#6730 * Add a path to suggest the correct opcode for the assembler
1 parent 0db9162 commit 199cb20

2 files changed

Lines changed: 29 additions & 0 deletions

File tree

source/text.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,18 @@ spv_result_t spvTextEncodeOpcode(const spvtools::AssemblyGrammar& grammar,
794794
if (spvOperandIsOptional(type)) {
795795
break;
796796
} else {
797+
if (opcodeName == "OpSpecConstantOp" &&
798+
type == SPV_OPERAND_TYPE_SPEC_CONSTANT_OP_NUMBER) {
799+
std::string operandValue;
800+
error = context->getWord(&operandValue, &nextPosition);
801+
spv::Op opcode;
802+
if (!grammar.lookupSpecConstantOpcode(operandValue.c_str() + 2,
803+
&opcode)) {
804+
return context->diagnostic()
805+
<< "Invalid " << opcodeName << " opcode '" << operandValue
806+
<< "'. Did you mean '" << operandValue.substr(2) << "'?";
807+
}
808+
}
797809
return context->diagnostic()
798810
<< "Expected operand for " << opcodeName
799811
<< " instruction, but found the next instruction instead.";

test/text_to_binary.constant_test.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,23 @@ INSTANTIATE_TEST_SUITE_P(
827827
{CASE(CompositeExtract), {0, 99, 42, 16, 17, 12, 19}},
828828
}));
829829

830+
using TextToBinaryOpSpecConstantOpNegative =
831+
spvtest::TextToBinaryTestBase<::testing::Test>;
832+
833+
TEST_F(TextToBinaryOpSpecConstantOpNegative, BadEnum) {
834+
std::string input = "%1 = OpSpecConstantOp %1 Add %3 %4";
835+
EXPECT_THAT(CompileFailure(input),
836+
testing::HasSubstr("Invalid OpSpecConstantOp opcode 'Add'"));
837+
}
838+
839+
TEST_F(TextToBinaryOpSpecConstantOpNegative, NearMiss) {
840+
std::string input = "%1 = OpSpecConstantOp %1 OpIAdd %3 %4";
841+
EXPECT_THAT(
842+
CompileFailure(input),
843+
testing::HasSubstr(
844+
"Invalid OpSpecConstantOp opcode 'OpIAdd'. Did you mean 'IAdd'?"));
845+
}
846+
830847
// TODO(dneto): OpConstantTrue
831848
// TODO(dneto): OpConstantFalse
832849
// TODO(dneto): OpConstantComposite

0 commit comments

Comments
 (0)