Skip to content
This repository was archived by the owner on Oct 11, 2025. It is now read-only.

Commit 0f03fe9

Browse files
JustinKim98Justin Kim
andauthored
[mlir][mlir-tblgen] Emit correct error message if method is pruned (#160334)
Add verification for pruned methods for `emitCustomBuilder` and `emitCheckedCustomBuilder` with proper diagnostic about shadowed methods. Without this verification, `mlir-tblgen` with `--gen-attrdef-decls` would segmentation fault if custom builder is provided with its body, but if method is pruned out due to duplication with other builders. Fixes #160227 --------- Co-authored-by: Justin Kim <[email protected]>
1 parent 6a23d8d commit 0f03fe9

File tree

2 files changed

+60
-9
lines changed

2 files changed

+60
-9
lines changed

mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -513,14 +513,57 @@ getCustomBuilderParams(std::initializer_list<MethodParameter> prefix,
513513
return builderParams;
514514
}
515515

516+
static std::string getSignature(const Method &m) {
517+
std::string signature;
518+
llvm::raw_string_ostream os(signature);
519+
raw_indented_ostream indentedOs(os);
520+
m.writeDeclTo(indentedOs);
521+
return signature;
522+
}
523+
524+
static void emitDuplicatedBuilderError(const Method &currentMethod,
525+
StringRef methodName,
526+
const Class &defCls,
527+
const AttrOrTypeDef &def) {
528+
529+
// Try to search for method that makes `get` redundant.
530+
auto loc = def.getDef()->getFieldLoc("builders");
531+
for (auto &method : defCls.getMethods()) {
532+
if (method->getName() == methodName &&
533+
method->makesRedundant(currentMethod)) {
534+
PrintError(loc, llvm::Twine("builder `") + methodName +
535+
"` conflicts with an existing builder. ");
536+
PrintFatalNote(llvm::Twine("A new builder with signature:\n") +
537+
getSignature(currentMethod) +
538+
"\nis shadowed by an existing builder with signature:\n" +
539+
getSignature(*method) +
540+
"\nPlease remove one of the conflicting "
541+
"definitions.");
542+
}
543+
}
544+
545+
// This code shouldn't be reached, but leaving this here for potential future
546+
// use.
547+
PrintFatalError(loc, "Failed to generate builder " + methodName);
548+
}
549+
516550
void DefGen::emitCustomBuilder(const AttrOrTypeBuilder &builder) {
517551
// Don't emit a body if there isn't one.
518552
auto props = builder.getBody() ? Method::Static : Method::StaticDeclaration;
519553
StringRef returnType = def.getCppClassName();
520554
if (std::optional<StringRef> builderReturnType = builder.getReturnType())
521555
returnType = *builderReturnType;
522-
Method *m = defCls.addMethod(returnType, "get", props,
523-
getCustomBuilderParams({}, builder));
556+
557+
llvm::StringRef methodName = "get";
558+
const auto parameters = getCustomBuilderParams({}, builder);
559+
Method *m = defCls.addMethod(returnType, methodName, props, parameters);
560+
561+
// If method is pruned, report error and terminate.
562+
if (!m) {
563+
auto curMethod = Method(returnType, methodName, props, parameters);
564+
emitDuplicatedBuilderError(curMethod, methodName, defCls, def);
565+
}
566+
524567
if (!builder.getBody())
525568
return;
526569

@@ -547,11 +590,19 @@ void DefGen::emitCheckedCustomBuilder(const AttrOrTypeBuilder &builder) {
547590
StringRef returnType = def.getCppClassName();
548591
if (std::optional<StringRef> builderReturnType = builder.getReturnType())
549592
returnType = *builderReturnType;
550-
Method *m = defCls.addMethod(
551-
returnType, "getChecked", props,
552-
getCustomBuilderParams(
553-
{{"::llvm::function_ref<::mlir::InFlightDiagnostic()>", "emitError"}},
554-
builder));
593+
594+
llvm::StringRef methodName = "getChecked";
595+
auto parameters = getCustomBuilderParams(
596+
{{"::llvm::function_ref<::mlir::InFlightDiagnostic()>", "emitError"}},
597+
builder);
598+
Method *m = defCls.addMethod(returnType, methodName, props, parameters);
599+
600+
// If method is pruned, report error and terminate.
601+
if (!m) {
602+
auto curMethod = Method(returnType, methodName, props, parameters);
603+
emitDuplicatedBuilderError(curMethod, methodName, defCls, def);
604+
}
605+
555606
if (!builder.getBody())
556607
return;
557608

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3104,8 +3104,8 @@ void OpEmitter::genBuilder() {
31043104
std::optional<StringRef> body = builder.getBody();
31053105
auto properties = body ? Method::Static : Method::StaticDeclaration;
31063106
auto *method = opClass.addMethod("void", "build", properties, arguments);
3107-
if (body)
3108-
ERROR_IF_PRUNED(method, "build", op);
3107+
3108+
ERROR_IF_PRUNED(method, "build", op);
31093109

31103110
if (method)
31113111
method->setDeprecated(builder.getDeprecatedMessage());

0 commit comments

Comments
 (0)