DiagnosticStream can convert to a stored error code

Use this to shorten error return code in the assembler.

For example, change this:

   if (error = something()) {
      diagnostic() << " Bad integer literal " << value;
      return SPV_ERROR_INVALID_VALUE;
   }

to this:

   if (error = something())
      return diagnostic() << " Bad integer literal " << value;

Also shorten code due to the fact that binaryEncodeU32 and
binaryCodeU64 can't fail (short of failure to expand a std::vector).
This commit is contained in:
David Neto 2015-10-09 15:48:09 -04:00
parent cc936dc613
commit ac508b0d80
6 changed files with 161 additions and 150 deletions

View File

@ -174,6 +174,7 @@ if (NOT ${SPIRV_SKIP_EXECUTABLES})
${CMAKE_CURRENT_SOURCE_DIR}/test/BinaryToText.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test/Comment.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test/DiagnosticPrint.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test/DiagnosticStream.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test/ExtInstGLSLstd450.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test/FixWord.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test/ImmediateInt.cpp

View File

@ -31,6 +31,7 @@
#include <iostream>
#include <sstream>
#include <utility>
class diagnostic_helper {
public:
@ -51,20 +52,26 @@ class diagnostic_helper {
spv_diagnostic *pDiagnostic;
};
// On destruction of the diagnostic stream, a diagnostic message will be
// written to pDiagnostic containing all of the data written to the stream.
// A DiagnosticStream remembers the current position of the input and an error
// code, and captures diagnostic messages via the left-shift operator.
// Captured messages are emitted during the destructor.
// TODO(awoloszyn): This is very similar to diagnostic_helper, and hides
// the data more easily. Replace diagnostic_helper elsewhere
// eventually.
class DiagnosticStream {
public:
DiagnosticStream(spv_position position, spv_diagnostic *pDiagnostic)
: position_(position), pDiagnostic_(pDiagnostic) {}
DiagnosticStream(spv_position position, spv_diagnostic *pDiagnostic,
spv_result_t error)
: position_(position), pDiagnostic_(pDiagnostic), error_(error) {}
DiagnosticStream(DiagnosticStream &&other) : position_(other.position_) {
stream_.str(other.stream_.str());
other.stream_.str("");
pDiagnostic_ = other.pDiagnostic_;
DiagnosticStream(DiagnosticStream &&other)
: stream_(other.stream_.str()),
position_(other.position_),
pDiagnostic_(other.pDiagnostic_),
error_(other.error_) {
// The other object's destructor will emit the text in its stream_
// member if its pDiagnostic_ member is non-null. Prevent that,
// since emitting that text is now the responsibility of *this.
other.pDiagnostic_ = nullptr;
}
@ -77,10 +84,14 @@ class DiagnosticStream {
return *this;
}
// Conversion operator to spv_result, returning the error code.
operator spv_result_t() { return error_; }
private:
std::stringstream stream_;
spv_position position_;
spv_diagnostic *pDiagnostic_;
spv_result_t error_;
};
#define DIAGNOSTIC \

View File

@ -166,12 +166,10 @@ spv_result_t encodeImmediate(libspirv::AssemblyContext* context,
const uint64_t parseResult = strtoull(begin, &end, 0);
size_t length = end - begin;
if (length != strlen(begin)) {
context->diagnostic() << "Invalid immediate integer '" << text << "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid immediate integer '" << text << "'.";
} else if (length > 10 || (parseResult >> 32) != 0) {
context->diagnostic() << "Immediate integer '" << text
return context->diagnostic() << "Immediate integer '" << text
<< "' is outside the unsigned 32-bit range.";
return SPV_ERROR_INVALID_TEXT;
}
context->binaryEncodeU32(parseResult, pInst);
context->seekForward(strlen(text));
@ -217,12 +215,10 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
if ('%' == textValue[0]) {
textValue++;
} else {
context->diagnostic() << "Expected id to start with %.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Expected id to start with %.";
}
if (!spvIsValidID(textValue)) {
context->diagnostic() << "Invalid ID " << textValue;
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid ID " << textValue;
}
const uint32_t id = context->spvNamedIdAssignOrGet(textValue);
if (type == SPV_OPERAND_TYPE_TYPE_ID) pInst->resultTypeId = id;
@ -233,9 +229,8 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
if (OpExtInst == pInst->opcode) {
spv_ext_inst_desc extInst;
if (grammar.lookupExtInst(pInst->extInstType, textValue, &extInst)) {
context->diagnostic() << "Invalid extended instruction name '"
<< textValue << "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid extended instruction name '"
<< textValue << "'.";
}
spvInstructionAddWord(pInst, extInst->ext_inst);
@ -253,9 +248,8 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
if (error != SPV_SUCCESS) {
if (error == SPV_ERROR_OUT_OF_MEMORY) return error;
if (spvOperandIsOptional(type)) return SPV_FAILED_MATCH;
context->diagnostic() << "Invalid literal number '" << textValue
<< "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid literal number '" << textValue
<< "'.";
}
// The encoding for OpConstant, OpSpecConstant and OpSwitch all
@ -272,20 +266,18 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
if (SPV_SUCCESS == grammar.lookupOpcode(pInst->opcode, &d)) {
opcode_name = d->name;
}
context->diagnostic()
<< "Type for " << opcode_name
<< " must be a scalar floating point or integer type";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic()
<< "Type for " << opcode_name
<< " must be a scalar floating point or integer type";
}
} else if (pInst->opcode == OpSwitch) {
// We need to know the value of the selector.
libspirv::IdType type =
context->getTypeOfValueInstruction(pInst->words[1]);
if (type.type_class != libspirv::IdTypeClass::kScalarIntegerType) {
context->diagnostic()
<< "The selector operand for OpSwitch must be the result"
" of an instruction that generates an integer scalar";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic()
<< "The selector operand for OpSwitch must be the result"
" of an instruction that generates an integer scalar";
}
}
// TODO(awoloszyn): Generate the correct assembly for arbitrary
@ -295,44 +287,36 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
// We do not have to print diagnostics here because spvBinaryEncode*
// prints diagnostic messages on failure.
case SPV_LITERAL_TYPE_INT_32:
if (context->binaryEncodeU32(BitwiseCast<uint32_t>(literal.value.i32),
pInst))
return SPV_ERROR_INVALID_TEXT;
context->binaryEncodeU32(BitwiseCast<uint32_t>(literal.value.i32),
pInst);
break;
case SPV_LITERAL_TYPE_INT_64: {
if (context->binaryEncodeU64(BitwiseCast<uint64_t>(literal.value.i64),
pInst))
return SPV_ERROR_INVALID_TEXT;
context->binaryEncodeU64(BitwiseCast<uint64_t>(literal.value.i64),
pInst);
} break;
case SPV_LITERAL_TYPE_UINT_32: {
if (context->binaryEncodeU32(literal.value.u32, pInst))
return SPV_ERROR_INVALID_TEXT;
context->binaryEncodeU32(literal.value.u32, pInst);
} break;
case SPV_LITERAL_TYPE_UINT_64: {
if (context->binaryEncodeU64(BitwiseCast<uint64_t>(literal.value.u64),
pInst))
return SPV_ERROR_INVALID_TEXT;
context->binaryEncodeU64(BitwiseCast<uint64_t>(literal.value.u64),
pInst);
} break;
case SPV_LITERAL_TYPE_FLOAT_32: {
if (context->binaryEncodeU32(BitwiseCast<uint32_t>(literal.value.f),
pInst))
return SPV_ERROR_INVALID_TEXT;
context->binaryEncodeU32(BitwiseCast<uint32_t>(literal.value.f),
pInst);
} break;
case SPV_LITERAL_TYPE_FLOAT_64: {
if (context->binaryEncodeU64(BitwiseCast<uint64_t>(literal.value.d),
pInst))
return SPV_ERROR_INVALID_TEXT;
context->binaryEncodeU64(BitwiseCast<uint64_t>(literal.value.d),
pInst);
} break;
case SPV_LITERAL_TYPE_STRING: {
context->diagnostic()
return context->diagnostic(SPV_FAILED_MATCH)
<< "Expected literal number, found literal string '" << textValue
<< "'.";
return SPV_FAILED_MATCH;
} break;
default:
context->diagnostic() << "Invalid literal number '" << textValue
<< "'";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid literal number '"
<< textValue << "'";
}
} break;
case SPV_OPERAND_TYPE_LITERAL_STRING:
@ -342,15 +326,13 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
if (error != SPV_SUCCESS) {
if (error == SPV_ERROR_OUT_OF_MEMORY) return error;
if (spvOperandIsOptional(type)) return SPV_FAILED_MATCH;
context->diagnostic() << "Invalid literal string '" << textValue
<< "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid literal string '" << textValue
<< "'.";
}
if (literal.type != SPV_LITERAL_TYPE_STRING) {
context->diagnostic()
<< "Expected literal string, found literal number '" << textValue
<< "'.";
return SPV_FAILED_MATCH;
return context->diagnostic(SPV_FAILED_MATCH)
<< "Expected literal string, found literal number '" << textValue
<< "'.";
}
// NOTE: Special case for extended instruction library import
@ -369,9 +351,8 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
case SPV_OPERAND_TYPE_SELECTION_CONTROL: {
uint32_t value;
if (grammar.parseMaskOperand(type, textValue, &value)) {
context->diagnostic() << "Invalid " << spvOperandTypeStr(type) << " '"
<< textValue << "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid " << spvOperandTypeStr(type)
<< " '" << textValue << "'.";
}
if (auto error = context->binaryEncodeU32(value, pInst)) return error;
// Prepare to parse the operands for this logical operand.
@ -394,9 +375,8 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
textValue, pInst, pExpectedOperands);
}
if (error) {
context->diagnostic() << "Invalid word following !<integer>: "
<< textValue;
return error;
return context->diagnostic(error)
<< "Invalid word following !<integer>: " << textValue;
}
if (pExpectedOperands->empty()) {
pExpectedOperands->push_back(SPV_OPERAND_TYPE_OPTIONAL_CIV);
@ -407,14 +387,12 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
// table.
spv_operand_desc entry;
if (grammar.lookupOperand(type, textValue, strlen(textValue), &entry)) {
context->diagnostic() << "Invalid " << spvOperandTypeStr(type) << " '"
<< textValue << "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid " << spvOperandTypeStr(type)
<< " '" << textValue << "'.";
}
if (context->binaryEncodeU32(entry->value, pInst)) {
context->diagnostic() << "Invalid " << spvOperandTypeStr(type) << " '"
<< textValue << "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid " << spvOperandTypeStr(type)
<< " '" << textValue << "'.";
}
// Prepare to parse the operands for this logical operand.
@ -437,10 +415,7 @@ spv_result_t encodeInstructionStartingWithImmediate(
std::string firstWord;
spv_position_t nextPosition = {};
auto error = context->getWord(firstWord, &nextPosition);
if (error) {
context->diagnostic() << "Internal Error";
return error;
}
if (error) return context->diagnostic(error) << "Internal Error";
if ((error = encodeImmediate(context, firstWord.c_str(), pInst))) {
return error;
@ -452,15 +427,11 @@ spv_result_t encodeInstructionStartingWithImmediate(
// Otherwise, there must be an operand that's either a literal, an ID, or
// an immediate.
std::string operandValue;
if ((error = context->getWord(operandValue, &nextPosition))) {
context->diagnostic() << "Internal Error";
return error;
}
if ((error = context->getWord(operandValue, &nextPosition)))
return context->diagnostic(error) << "Internal Error";
if (operandValue == "=") {
context->diagnostic() << firstWord << " not allowed before =.";
return SPV_ERROR_INVALID_TEXT;
}
if (operandValue == "=")
return context->diagnostic() << firstWord << " not allowed before =.";
// Needed to pass to spvTextEncodeOpcode(), but it shouldn't ever be
// expanded.
@ -501,10 +472,7 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar,
std::string firstWord;
spv_position_t nextPosition = {};
spv_result_t error = context->getWord(firstWord, &nextPosition);
if (error) {
context->diagnostic() << "Internal Error";
return error;
}
if (error) return context->diagnostic() << "Internal Error";
std::string opcodeName;
std::string result_id;
@ -515,49 +483,39 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar,
// If the first word of this instruction is not an opcode, we must be
// processing AAF now.
if (SPV_ASSEMBLY_SYNTAX_FORMAT_ASSIGNMENT != format) {
context->diagnostic()
<< "Expected <opcode> at the beginning of an instruction, found '"
<< firstWord << "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic()
<< "Expected <opcode> at the beginning of an instruction, found '"
<< firstWord << "'.";
}
result_id = firstWord;
if ('%' != result_id.front()) {
context->diagnostic()
<< "Expected <opcode> or <result-id> at the beginning "
"of an instruction, found '"
<< result_id << "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic()
<< "Expected <opcode> or <result-id> at the beginning "
"of an instruction, found '"
<< result_id << "'.";
}
result_id_position = context->position();
// The '=' sign.
context->setPosition(nextPosition);
if (context->advance()) {
context->diagnostic() << "Expected '=', found end of stream.";
return SPV_ERROR_INVALID_TEXT;
}
if (context->advance())
return context->diagnostic() << "Expected '=', found end of stream.";
std::string equal_sign;
error = context->getWord(equal_sign, &nextPosition);
if ("=" != equal_sign) {
context->diagnostic() << "'=' expected after result id.";
return SPV_ERROR_INVALID_TEXT;
}
if ("=" != equal_sign)
return context->diagnostic() << "'=' expected after result id.";
// The <opcode> after the '=' sign.
context->setPosition(nextPosition);
if (context->advance()) {
context->diagnostic() << "Expected opcode, found end of stream.";
return SPV_ERROR_INVALID_TEXT;
}
if (context->advance())
return context->diagnostic() << "Expected opcode, found end of stream.";
error = context->getWord(opcodeName, &nextPosition);
if (error) {
context->diagnostic() << "Internal Error";
return error;
}
if (error)
return context->diagnostic(error) << "Internal Error";
if (!context->startsWithOp()) {
context->diagnostic() << "Invalid Opcode prefix '" << opcodeName << "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic() << "Invalid Opcode prefix '" << opcodeName
<< "'.";
}
}
@ -567,17 +525,16 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar,
spv_opcode_desc opcodeEntry;
error = grammar.lookupOpcode(pInstName, &opcodeEntry);
if (error) {
context->diagnostic() << "Invalid Opcode name '" << context->getWord()
<< "'";
return error;
return context->diagnostic(error) << "Invalid Opcode name '"
<< context->getWord() << "'";
}
if (SPV_ASSEMBLY_SYNTAX_FORMAT_ASSIGNMENT == format) {
// If this instruction has <result-id>, check it follows AAF.
if (opcodeEntry->hasResult && result_id.empty()) {
context->diagnostic() << "Expected <result-id> at the beginning of an "
"instruction, found '"
<< firstWord << "'.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic()
<< "Expected <result-id> at the beginning of an "
"instruction, found '"
<< firstWord << "'.";
}
}
pInst->opcode = opcodeEntry->opcode;
@ -624,8 +581,8 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar,
// and we didn't find one. We're finished parsing this instruction.
break;
} else {
context->diagnostic() << "Expected operand, found end of stream.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic()
<< "Expected operand, found end of stream.";
}
}
assert(error == SPV_SUCCESS && "Somebody added another way to fail");
@ -634,18 +591,14 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar,
if (spvOperandIsOptional(type)) {
break;
} else {
context->diagnostic()
<< "Expected operand, found next instruction instead.";
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic()
<< "Expected operand, found next instruction instead.";
}
}
std::string operandValue;
error = context->getWord(operandValue, &nextPosition);
if (error) {
context->diagnostic() << "Internal Error";
return error;
}
if (error) return context->diagnostic(error) << "Internal Error";
error = spvTextEncodeOperand(grammar, context, type, operandValue.c_str(),
pInst, &expectedOperands);
@ -672,10 +625,10 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar,
}
if (pInst->words.size() > SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX) {
context->diagnostic() << "Instruction too long: " << pInst->words.size()
<< " words, but the limit is "
<< SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX;
return SPV_ERROR_INVALID_TEXT;
return context->diagnostic()
<< "Instruction too long: " << pInst->words.size()
<< " words, but the limit is "
<< SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX;
}
pInst->words[0] = spvOpcodeMake(pInst->words.size(), opcodeEntry->opcode);
@ -695,10 +648,9 @@ spv_result_t spvTextToBinaryInternal(const libspirv::AssemblyGrammar& grammar,
spv_diagnostic* pDiagnostic) {
if (!pDiagnostic) return SPV_ERROR_INVALID_DIAGNOSTIC;
libspirv::AssemblyContext context(text, pDiagnostic);
if (!text->str || !text->length) {
context.diagnostic() << "Text stream is empty.";
return SPV_ERROR_INVALID_TEXT;
}
if (!text->str || !text->length)
return context.diagnostic() << "Text stream is empty.";
if (!grammar.isValid()) {
return SPV_ERROR_INVALID_TABLE;
}
@ -709,10 +661,7 @@ spv_result_t spvTextToBinaryInternal(const libspirv::AssemblyGrammar& grammar,
std::vector<spv_instruction_t> instructions;
if (context.advance()) {
context.diagnostic() << "Text stream is empty.";
return SPV_ERROR_INVALID_TEXT;
}
if (context.advance()) return context.diagnostic() << "Text stream is empty.";
spv_ext_inst_type_t extInstType = SPV_EXT_INST_TYPE_NONE;
while (context.hasText()) {

View File

@ -333,11 +333,9 @@ spv_result_t AssemblyContext::binaryEncodeU64(const uint64_t value,
spv_instruction_t *pInst) {
uint32_t low = (uint32_t)(0x00000000ffffffff & value);
uint32_t high = (uint32_t)((0xffffffff00000000 & value) >> 32);
spv_result_t err = binaryEncodeU32(low, pInst);
if (err != SPV_SUCCESS) {
return err;
}
return binaryEncodeU32(high, pInst);
binaryEncodeU32(low, pInst);
binaryEncodeU32(high, pInst);
return SPV_SUCCESS;
}
spv_result_t AssemblyContext::binaryEncodeString(

View File

@ -168,10 +168,16 @@ class AssemblyContext {
bool isStartOfNewInst();
// Returns a diagnostic object initialized with current position in the input
// stream. Any data written to this object will show up in pDiagnsotic on
// destruction.
// stream, and for the given error code. Any data written to this object will
// show up in pDiagnsotic on destruction.
DiagnosticStream diagnostic(spv_result_t error) {
return DiagnosticStream(&current_position_, pDiagnostic_, error);
}
// Returns a diagnostic object with the default assembly error code.
DiagnosticStream diagnostic() {
return DiagnosticStream(&current_position_, pDiagnostic_);
// The default failure for assembly is invalid text.
return diagnostic(SPV_ERROR_INVALID_TEXT);
}
// Returns then next characted in the input stream.

46
test/DiagnosticStream.cpp Normal file
View File

@ -0,0 +1,46 @@
// Copyright (c) 2015 The Khronos Group Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and/or associated documentation files (the
// "Materials"), to deal in the Materials without restriction, including
// without limitation the rights to use, copy, modify, merge, publish,
// distribute, sublicense, and/or sell copies of the Materials, and to
// permit persons to whom the Materials are furnished to do so, subject to
// the following conditions:
//
// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Materials.
//
// MODIFICATIONS TO THIS FILE MAY MEAN IT NO LONGER ACCURATELY REFLECTS
// KHRONOS STANDARDS. THE UNMODIFIED, NORMATIVE VERSIONS OF KHRONOS
// SPECIFICATIONS AND HEADER INFORMATION ARE LOCATED AT
// https://www.khronos.org/registry/
//
// THE MATERIALS ARE PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
// IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
// TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
// MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS.
#include "UnitSPIRV.h"
namespace {
TEST(DiagnosticStream, ConversionToResultType) {
// Check after the DiagnosticStream object is destroyed.
spv_result_t value;
{ value = DiagnosticStream(0, 0, SPV_ERROR_INVALID_TEXT); }
EXPECT_EQ(SPV_ERROR_INVALID_TEXT, value);
// Check implicit conversion via plain assignment.
value = DiagnosticStream(0, 0, SPV_SUCCESS);
EXPECT_EQ(SPV_SUCCESS, value);
// Check conversion via constructor.
EXPECT_EQ(SPV_FAILED_MATCH,
spv_result_t(DiagnosticStream(0, 0, SPV_FAILED_MATCH)));
}
} // anonymous namespace