[ParseResult] Mark this as LLVM_NODISCARD (like LogicalResult) and fix issues.

There are a lot of cases where we accidentally ignored the result of some
parsing hook.  Mark ParseResult as LLVM_NODISCARD just like ParseResult is.
This exposed some stuff to clean up, so do.

Differential Revision: https://reviews.llvm.org/D125549
This commit is contained in:
Chris Lattner 2022-05-13 15:38:50 +01:00
parent 0d67c8a51d
commit 1d7b5cd5bf
17 changed files with 78 additions and 83 deletions

View File

@ -109,7 +109,7 @@ private:
/// swallowed up in boilerplate without this, so we provide this for narrow
/// cases where it is important.
///
class ParseResult : public LogicalResult {
class LLVM_NODISCARD ParseResult : public LogicalResult {
public:
ParseResult(LogicalResult result = success()) : LogicalResult(result) {}

View File

@ -3382,7 +3382,7 @@ void AffineParallelOp::print(OpAsmPrinter &p) {
/// unique operands. Also populates `replacements with affine expressions of
/// `kind` that can be used to update affine maps previously accepting a
/// `operands` to accept `uniqueOperands` instead.
static void deduplicateAndResolveOperands(
static ParseResult deduplicateAndResolveOperands(
OpAsmParser &parser,
ArrayRef<SmallVector<OpAsmParser::UnresolvedOperand>> operands,
SmallVectorImpl<Value> &uniqueOperands,
@ -3393,7 +3393,8 @@ static void deduplicateAndResolveOperands(
Type indexType = parser.getBuilder().getIndexType();
for (const auto &list : operands) {
SmallVector<Value> valueOperands;
parser.resolveOperands(list, indexType, valueOperands);
if (parser.resolveOperands(list, indexType, valueOperands))
return failure();
for (Value operand : valueOperands) {
unsigned pos = std::distance(uniqueOperands.begin(),
llvm::find(uniqueOperands, operand));
@ -3405,6 +3406,7 @@ static void deduplicateAndResolveOperands(
: getAffineSymbolExpr(pos, parser.getContext()));
}
}
return success();
}
namespace {
@ -3502,10 +3504,11 @@ static ParseResult parseAffineMapWithMinMax(OpAsmParser &parser,
// Deduplicate map operands.
SmallVector<Value> dimOperands, symOperands;
SmallVector<AffineExpr> dimRplacements, symRepacements;
deduplicateAndResolveOperands(parser, flatDimOperands, dimOperands,
dimRplacements, AffineExprKind::DimId);
deduplicateAndResolveOperands(parser, flatSymOperands, symOperands,
symRepacements, AffineExprKind::SymbolId);
if (deduplicateAndResolveOperands(parser, flatDimOperands, dimOperands,
dimRplacements, AffineExprKind::DimId) ||
deduplicateAndResolveOperands(parser, flatSymOperands, symOperands,
symRepacements, AffineExprKind::SymbolId))
return failure();
result.operands.append(dimOperands.begin(), dimOperands.end());
result.operands.append(symOperands.begin(), symOperands.end());

View File

@ -210,17 +210,15 @@ ParseResult ExecuteOp::parse(OpAsmParser &parser, OperationState &result) {
// Parse the types of results returned from the async execute op.
SmallVector<Type, 4> resultTypes;
if (parser.parseOptionalArrowTypeList(resultTypes))
return failure();
// Async execute first result is always a completion token.
parser.addTypeToList(tokenTy, result.types);
parser.addTypesToList(resultTypes, result.types);
// Parse operation attributes.
NamedAttrList attrs;
if (parser.parseOptionalAttrDictWithKeyword(attrs))
if (parser.parseOptionalArrowTypeList(resultTypes) ||
// Async execute first result is always a completion token.
parser.addTypeToList(tokenTy, result.types) ||
parser.addTypesToList(resultTypes, result.types) ||
// Parse operation attributes.
parser.parseOptionalAttrDictWithKeyword(attrs))
return failure();
result.addAttributes(attrs);
// Parse asynchronous region.

View File

@ -540,7 +540,8 @@ parseGEPIndices(OpAsmParser &parser,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &indices,
DenseIntElementsAttr &structIndices) {
SmallVector<int32_t> constantIndices;
parser.parseCommaSeparatedList([&]() -> ParseResult {
auto idxParser = [&]() -> ParseResult {
int32_t constantIndex;
OptionalParseResult parsedInteger =
parser.parseOptionalInteger(constantIndex);
@ -553,7 +554,9 @@ parseGEPIndices(OpAsmParser &parser,
constantIndices.push_back(LLVM::GEPOp::kDynamicIndex);
return parser.parseOperand(indices.emplace_back());
});
};
if (parser.parseCommaSeparatedList(idxParser))
return failure();
structIndices = parser.getBuilder().getI32TensorAttr(constantIndices);
return success();

View File

@ -306,8 +306,8 @@ ParseResult MmaOp::parse(OpAsmParser &parser, OperationState &result) {
}
Type resultType;
parser.parseArrow();
parser.parseType(resultType);
if (parser.parseArrow() || parser.parseType(resultType))
return failure();
frags[3].elemtype = inferOperandMMAType(resultType, /*isAccum=*/true);
std::array<StringRef, 2> names{"multiplicandAPtxType",

View File

@ -130,7 +130,8 @@ parseCommonStructuredOpParts(OpAsmParser &parser, OperationState &result,
SmallVector<OpAsmParser::UnresolvedOperand, 4> inputsOperands,
outputsOperands;
parser.parseOptionalAttrDict(result.attributes);
if (parser.parseOptionalAttrDict(result.attributes))
return failure();
if (succeeded(parser.parseOptionalKeyword("ins"))) {
if (parser.parseLParen())

View File

@ -118,11 +118,9 @@ ParseResult transform::TileOp::parse(OpAsmParser &parser,
OperationState &result) {
StringRef sizesAttrName = TileOp::getSizesAttrName(result.name).getValue();
OpAsmParser::UnresolvedOperand targetOperand;
SMLoc opLoc;
parser.getCurrentLocation(&opLoc);
if (parser.parseOperand(targetOperand))
return parser.emitError(opLoc, "expected 'target' operand");
if (parser.parseOptionalAttrDict(result.attributes))
SMLoc opLoc = parser.getCurrentLocation();
if (parser.parseOperand(targetOperand) ||
parser.parseOptionalAttrDict(result.attributes))
return failure();
Attribute sizesAttr = result.attributes.get(sizesAttrName);
if (!sizesAttr)

View File

@ -154,8 +154,9 @@ static OptionalParseResult parseOptionalOperandAndType(OpAsmParser &parser,
static OptionalParseResult parserOptionalOperandAndTypeWithPrefix(
OpAsmParser &parser, OperationState &result, StringRef prefixKeyword) {
if (succeeded(parser.parseOptionalKeyword(prefixKeyword))) {
parser.parseEqual();
return parseOperandAndType(parser, result);
if (parser.parseEqual() || parseOperandAndType(parser, result))
return failure();
return success();
}
return llvm::None;
}
@ -532,12 +533,14 @@ ParseResult LoopOp::parse(OpAsmParser &parser, OperationState &result) {
parser, result, LoopOp::getGangNumKeyword());
if (gangNum.hasValue() && failed(*gangNum))
return failure();
parser.parseOptionalComma();
// FIXME: Comma should require subsequent operands.
(void)parser.parseOptionalComma();
gangStatic = parserOptionalOperandAndTypeWithPrefix(
parser, result, LoopOp::getGangStaticKeyword());
if (gangStatic.hasValue() && failed(*gangStatic))
return failure();
parser.parseOptionalComma();
// FIXME: Why allow optional last commas?
(void)parser.parseOptionalComma();
if (failed(parser.parseRParen()))
return failure();
}

View File

@ -31,10 +31,8 @@ static IntegerType parseStorageType(DialectAsmParser &parser, bool &isSigned) {
unsigned storageTypeWidth = 0;
OptionalParseResult result = parser.parseOptionalType(type);
if (result.hasValue()) {
if (!succeeded(*result)) {
parser.parseType(type);
if (!succeeded(*result))
return nullptr;
}
isSigned = !type.isUnsigned();
storageTypeWidth = type.getWidth();
} else if (succeeded(parser.parseKeyword(&identifier))) {

View File

@ -2032,15 +2032,13 @@ ParseResult ParallelOp::parse(OpAsmParser &parser, OperationState &result) {
static_cast<int32_t>(initVals.size())}));
// Parse attributes.
if (parser.parseOptionalAttrDict(result.attributes))
if (parser.parseOptionalAttrDict(result.attributes) ||
parser.resolveOperands(initVals, result.types, parser.getNameLoc(),
result.operands))
return failure();
if (!initVals.empty())
parser.resolveOperands(initVals, result.types, parser.getNameLoc(),
result.operands);
// Add a terminator if none was parsed.
ForOp::ensureTerminator(*body, builder, result.location);
return success();
}

View File

@ -3112,15 +3112,15 @@ void spirv::ModuleOp::build(OpBuilder &builder, OperationState &state,
ParseResult spirv::ModuleOp::parse(OpAsmParser &parser, OperationState &state) {
Region *body = state.addRegion();
StringAttr nameAttr;
spirv::AddressingModel addrModel;
spirv::MemoryModel memoryModel;
// If the name is present, parse it.
StringAttr nameAttr;
parser.parseOptionalSymbolName(
(void)parser.parseOptionalSymbolName(
nameAttr, mlir::SymbolTable::getSymbolAttrName(), state.attributes);
// Parse attributes
spirv::AddressingModel addrModel;
spirv::MemoryModel memoryModel;
if (::parseEnumKeywordAttr(addrModel, parser, state) ||
::parseEnumKeywordAttr(memoryModel, parser, state))
return failure();
@ -3133,10 +3133,8 @@ ParseResult spirv::ModuleOp::parse(OpAsmParser &parser, OperationState &state) {
return failure();
}
if (parser.parseOptionalAttrDictWithKeyword(state.attributes))
return failure();
if (parser.parseRegion(*body, /*arguments=*/{}, /*argTypes=*/{}))
if (parser.parseOptionalAttrDictWithKeyword(state.attributes) ||
parser.parseRegion(*body, /*arguments=*/{}, /*argTypes=*/{}))
return failure();
// Make sure we have at least one block.

View File

@ -2864,7 +2864,8 @@ ParseResult TransferReadOp::parse(OpAsmParser &parser, OperationState &result) {
return failure();
ParseResult hasMask = parser.parseOptionalComma();
if (hasMask.succeeded()) {
parser.parseOperand(maskInfo);
if (parser.parseOperand(maskInfo))
return failure();
}
if (parser.parseOptionalAttrDict(result.attributes) ||
parser.getCurrentLocation(&typesLoc) || parser.parseColonTypeList(types))

View File

@ -170,7 +170,7 @@ ParseResult mlir::function_interface_impl::parseFunctionOp(
auto &builder = parser.getBuilder();
// Parse visibility.
impl::parseOptionalVisibilityKeyword(parser, result.attributes);
(void)impl::parseOptionalVisibilityKeyword(parser, result.attributes);
// Parse the name as a symbol.
StringAttr nameAttr;

View File

@ -50,9 +50,11 @@ public:
: Parser(state), allowParsingSSAIds(allowParsingSSAIds),
parseElement(parseElement) {}
AffineMap parseAffineMapRange(unsigned numDims, unsigned numSymbols);
ParseResult parseAffineMapRange(unsigned numDims, unsigned numSymbols,
AffineMap &result);
ParseResult parseAffineMapOrIntegerSetInline(AffineMap &map, IntegerSet &set);
IntegerSet parseIntegerSetConstraints(unsigned numDims, unsigned numSymbols);
ParseResult parseIntegerSetConstraints(unsigned numDims, unsigned numSymbols,
IntegerSet &result);
ParseResult parseAffineMapOfSSAIds(AffineMap &map,
OpAsmParser::Delimiter delimiter);
ParseResult parseAffineExprOfSSAIds(AffineExpr &expr);
@ -510,29 +512,15 @@ ParseResult AffineParser::parseAffineMapOrIntegerSetInline(AffineMap &map,
unsigned numDims = 0, numSymbols = 0;
// List of dimensional and optional symbol identifiers.
if (parseDimAndOptionalSymbolIdList(numDims, numSymbols)) {
return failure();
}
// This is needed for parsing attributes as we wouldn't know whether we would
// be parsing an integer set attribute or an affine map attribute.
bool isArrow = getToken().is(Token::arrow);
bool isColon = getToken().is(Token::colon);
if (!isArrow && !isColon)
return emitWrongTokenError("expected '->' or ':'");
if (isArrow) {
parseToken(Token::arrow, "expected '->' or '['");
map = parseAffineMapRange(numDims, numSymbols);
return map ? success() : failure();
}
if (parseToken(Token::colon, "expected ':' or '['"))
if (parseDimAndOptionalSymbolIdList(numDims, numSymbols))
return failure();
if ((set = parseIntegerSetConstraints(numDims, numSymbols)))
return success();
if (consumeIf(Token::arrow))
return parseAffineMapRange(numDims, numSymbols, map);
return failure();
if (parseToken(Token::colon, "expected '->' or '['"))
return failure();
return parseIntegerSetConstraints(numDims, numSymbols, set);
}
/// Parse an AffineMap where the dim and symbol identifiers are SSA ids.
@ -572,8 +560,9 @@ ParseResult AffineParser::parseAffineExprOfSSAIds(AffineExpr &expr) {
///
/// multi-dim-affine-expr ::= `(` `)`
/// multi-dim-affine-expr ::= `(` affine-expr (`,` affine-expr)* `)`
AffineMap AffineParser::parseAffineMapRange(unsigned numDims,
unsigned numSymbols) {
ParseResult AffineParser::parseAffineMapRange(unsigned numDims,
unsigned numSymbols,
AffineMap &result) {
SmallVector<AffineExpr, 4> exprs;
auto parseElt = [&]() -> ParseResult {
auto elt = parseAffineExpr();
@ -588,10 +577,11 @@ AffineMap AffineParser::parseAffineMapRange(unsigned numDims,
// | `(` affine-expr (`,` affine-expr)* `)`
if (parseCommaSeparatedList(Delimiter::Paren, parseElt,
" in affine map range"))
return AffineMap();
return failure();
// Parsed a valid affine map.
return AffineMap::get(numDims, numSymbols, exprs, getContext());
result = AffineMap::get(numDims, numSymbols, exprs, getContext());
return success();
}
/// Parse an affine constraint.
@ -639,8 +629,9 @@ AffineExpr AffineParser::parseAffineConstraint(bool *isEq) {
/// affine-constraint-conjunction ::= affine-constraint (`,`
/// affine-constraint)*
///
IntegerSet AffineParser::parseIntegerSetConstraints(unsigned numDims,
unsigned numSymbols) {
ParseResult AffineParser::parseIntegerSetConstraints(unsigned numDims,
unsigned numSymbols,
IntegerSet &result) {
SmallVector<AffineExpr, 4> constraints;
SmallVector<bool, 4> isEqs;
auto parseElt = [&]() -> ParseResult {
@ -657,17 +648,19 @@ IntegerSet AffineParser::parseIntegerSetConstraints(unsigned numDims,
// Parse a list of affine constraints (comma-separated).
if (parseCommaSeparatedList(Delimiter::Paren, parseElt,
" in integer set constraint list"))
return IntegerSet();
return failure();
// If no constraints were parsed, then treat this as a degenerate 'true' case.
if (constraints.empty()) {
/* 0 == 0 */
auto zero = getAffineConstantExpr(0, getContext());
return IntegerSet::get(numDims, numSymbols, zero, true);
result = IntegerSet::get(numDims, numSymbols, zero, true);
return success();
}
// Parsed a valid integer set.
return IntegerSet::get(numDims, numSymbols, constraints, isEqs);
result = IntegerSet::get(numDims, numSymbols, constraints, isEqs);
return success();
}
//===----------------------------------------------------------------------===//

View File

@ -199,7 +199,7 @@ func.func @parallel_more_results_than_reduces(
func.func @parallel_more_results_than_initial_values(
%arg0 : index, %arg1: index, %arg2: index) {
// expected-error@+1 {{expects number of results: 1 to be the same as number of initial values: 0}}
// expected-error@+1 {{'scf.parallel' 0 operands present, but expected 1}}
%res = scf.parallel (%i0) = (%arg0) to (%arg1) step (%arg2) -> f32 {
scf.reduce(%arg0) : index {
^bb0(%lhs: index, %rhs: index):

View File

@ -932,7 +932,7 @@ func.func @invalid_tensor_literal() {
func.func @invalid_affine_structure() {
%c0 = arith.constant 0 : index
%idx = affine.apply affine_map<(d0, d1)> (%c0, %c0) // expected-error {{expected '->' or ':'}}
%idx = affine.apply affine_map<(d0, d1)> (%c0, %c0) // expected-error {{expected '->' or '['}}
return
}

View File

@ -212,12 +212,13 @@ struct DLTestDialect : Dialect {
return CustomDataLayoutSpec::get(parser.getContext(), {});
SmallVector<DataLayoutEntryInterface> entries;
parser.parseCommaSeparatedList([&]() {
ok = succeeded(parser.parseCommaSeparatedList([&]() {
entries.emplace_back();
ok = succeeded(parser.parseAttribute(entries.back()));
assert(ok);
return success();
});
}));
assert(ok);
ok = succeeded(parser.parseGreater());
assert(ok);
return CustomDataLayoutSpec::get(parser.getContext(), entries);