[Mlir-commits] [llvm] [mlir] [TableGen] Assert that RecordVal values are never null (PR #235077)
Jay Foad
llvmlistbot at llvm.org
Fri Oct 23 12:51:52 PDT 2026
https://github.com/jayfoad created https://github.com/llvm/llvm-project/pull/235077
A RecordVal always has a value: it starts as an UnsetInit and is only
ever set to another non-null value. Replace the dead null cases in
RecordVal::setValue and TGParser::SetValue with assertions, document the
invariant on RecordVal::getValue, and remove the redundant null checks
on RecordVal values in TableGen and in LLVM and MLIR backends. NFC.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply at anthropic.com>
>From d40eb091ec7b1c0b21cf591ce89c810c48ae9f51 Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Fri, 23 Oct 2026 18:20:50 +0100
Subject: [PATCH] [TableGen] Assert that RecordVal values are never null
A RecordVal always has a value: it starts as an UnsetInit and is only
ever set to another non-null value. Replace the dead null cases in
RecordVal::setValue and TGParser::SetValue with assertions, document the
invariant on RecordVal::getValue, and remove the redundant null checks
on RecordVal values in TableGen and in LLVM and MLIR backends. NFC.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply at anthropic.com>
---
llvm/include/llvm/TableGen/Record.h | 3 +-
llvm/lib/TableGen/Record.cpp | 43 ++++++++-----------
llvm/lib/TableGen/TGParser.cpp | 19 ++++----
llvm/utils/TableGen/AsmMatcherEmitter.cpp | 2 +-
.../TableGen/Common/SubtargetFeatureInfo.cpp | 2 +-
llvm/utils/TableGen/DXILEmitter.cpp | 2 +-
llvm/utils/TableGen/RegisterInfoEmitter.cpp | 2 +-
mlir/lib/TableGen/Attribute.cpp | 2 +-
mlir/lib/TableGen/Builder.cpp | 2 +-
mlir/lib/TableGen/Property.cpp | 2 +-
mlir/lib/TableGen/Type.cpp | 2 +-
.../tblgen-lsp-server/TableGenServer.cpp | 2 +-
12 files changed, 36 insertions(+), 47 deletions(-)
diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
index 679a3b81aa2885..5636776521edcd 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -1643,7 +1643,8 @@ class RecordVal {
/// Get the type of the field for printing purposes.
std::string getPrintType() const;
- /// Get the value of the field as an Init.
+ /// Get the value of the field as an Init. This is never null: a field that
+ /// has not been given a value has an UnsetInit.
const Init *getValue() const { return Value; }
/// Set the value of the field from an Init.
diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index 2667e580425312..768912c279f6d2 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -2765,11 +2765,7 @@ std::string RecordVal::getPrintType() const {
}
bool RecordVal::setValue(const Init *V) {
- if (!V) {
- Value = nullptr;
- return false;
- }
-
+ assert(V && "Cannot set a null value");
const Init *NewValue = V->getCastTo(getType());
if (!NewValue)
return true;
@@ -2802,10 +2798,7 @@ LLVM_DUMP_METHOD void RecordVal::dump() const { errs() << *this; }
void RecordVal::print(raw_ostream &OS, bool PrintSem) const {
if (isNonconcreteOK()) OS << "field ";
- OS << getPrintType() << " " << getNameInitAsString();
-
- if (getValue())
- OS << " = " << *getValue();
+ OS << getPrintType() << " " << getNameInitAsString() << " = " << *getValue();
if (PrintSem) OS << ";\n";
}
@@ -2872,21 +2865,19 @@ void Record::resolveReferences(Resolver &R, const RecordVal *SkipVal) {
for (RecordVal &Value : Values) {
if (SkipVal == &Value) // Skip resolve the same field as the given one
continue;
- if (const Init *V = Value.getValue()) {
- const Init *VR = V->resolveReferences(R);
- if (Value.setValue(VR)) {
- std::string Type;
- if (const auto *VRT = dyn_cast<TypedInit>(VR))
- Type =
- (Twine("of type '") + VRT->getType()->getAsString() + "' ").str();
- PrintFatalError(
- getLoc(),
- Twine("Invalid value ") + Type + "found when setting field '" +
- Value.getNameInitAsString() + "' of type '" +
- Value.getType()->getAsString() +
- "' after resolving references: " + VR->getAsUnquotedString() +
- "\n");
- }
+ const Init *VR = Value.getValue()->resolveReferences(R);
+ if (Value.setValue(VR)) {
+ std::string Type;
+ if (const auto *VRT = dyn_cast<TypedInit>(VR))
+ Type =
+ (Twine("of type '") + VRT->getType()->getAsString() + "' ").str();
+ PrintFatalError(
+ getLoc(),
+ Twine("Invalid value ") + Type + "found when setting field '" +
+ Value.getNameInitAsString() + "' of type '" +
+ Value.getType()->getAsString() +
+ "' after resolving references: " + VR->getAsUnquotedString() +
+ "\n");
}
}
@@ -2960,7 +2951,7 @@ SMLoc Record::getFieldLoc(StringRef FieldName) const {
const Init *Record::getValueInit(StringRef FieldName) const {
const RecordVal *R = getValue(FieldName);
- if (!R || !R->getValue())
+ if (!R)
PrintFatalError(getLoc(), "Record `" + getName() +
"' does not have a field named `" + FieldName + "'!\n");
return R->getValue();
@@ -2977,7 +2968,7 @@ StringRef Record::getValueAsString(StringRef FieldName) const {
std::optional<StringRef>
Record::getValueAsOptionalString(StringRef FieldName) const {
const RecordVal *R = getValue(FieldName);
- if (!R || !R->getValue())
+ if (!R)
return std::nullopt;
if (isa<UnsetInit>(R->getValue()))
return std::nullopt;
diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index 00e691a87d3938..aea15665bff06d 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -104,15 +104,13 @@ static void checkConcrete(Record &R) {
if (RV.isNonconcreteOK())
continue;
- if (const Init *V = RV.getValue()) {
- bool Ok = isa<BitsInit>(V) ? checkBitsConcrete(R, RV) : V->isConcrete();
- if (!Ok) {
- PrintError(R.getLoc(), Twine("Initializer of '") +
- RV.getNameInitAsString() + "' in '" +
- R.getNameInitAsString() +
- "' could not be fully resolved: " +
- RV.getValue()->getAsString());
- }
+ const Init *V = RV.getValue();
+ bool Ok = isa<BitsInit>(V) ? checkBitsConcrete(R, RV) : V->isConcrete();
+ if (!Ok) {
+ PrintError(R.getLoc(),
+ Twine("Initializer of '") + RV.getNameInitAsString() +
+ "' in '" + R.getNameInitAsString() +
+ "' could not be fully resolved: " + V->getAsString());
}
}
}
@@ -244,8 +242,7 @@ bool TGParser::AddValue(Record *CurRec, SMLoc Loc, const RecordVal &RV,
bool TGParser::SetValue(Record *CurRec, SMLoc Loc, const Init *ValName,
ArrayRef<unsigned> BitList, const Init *V,
bool OverrideDefLoc, LetMode Mode) {
- if (!V)
- return false;
+ assert(V && "Cannot set a null value");
if (!CurRec)
CurRec = &CurMultiClass->Rec;
diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index b756582603ffdf..50de2288bc6028 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -1215,7 +1215,7 @@ ClassInfo *AsmMatcherInfo::getOperandClass(const Record *Rec, int SubOpIdx) {
// RegisterOperand may have an associated ParserMatchClass. If it does,
// use it, else just fall back to the underlying register class.
const RecordVal *R = Rec->getValue("ParserMatchClass");
- if (!R || !R->getValue())
+ if (!R)
PrintFatalError(Rec->getLoc(),
"Record `" + Rec->getName() +
"' does not have a ParserMatchClass!\n");
diff --git a/llvm/utils/TableGen/Common/SubtargetFeatureInfo.cpp b/llvm/utils/TableGen/Common/SubtargetFeatureInfo.cpp
index 74942397c2e8d1..3be1e3c98138fa 100644
--- a/llvm/utils/TableGen/Common/SubtargetFeatureInfo.cpp
+++ b/llvm/utils/TableGen/Common/SubtargetFeatureInfo.cpp
@@ -222,7 +222,7 @@ bool llvm::getRequiredFeatures(
continue;
}
const RecordVal *V = R->getValue("AssemblerCondDag");
- if (!V || !V->getValue() || !isa<DagInit>(V->getValue())) {
+ if (!V || !isa<DagInit>(V->getValue())) {
// Ignore this predicate for semantic analysis, as it has no subtarget
// features.
AllPredicatesParsed = false;
diff --git a/llvm/utils/TableGen/DXILEmitter.cpp b/llvm/utils/TableGen/DXILEmitter.cpp
index 94719598dfd58e..3606e02cb9b099 100644
--- a/llvm/utils/TableGen/DXILEmitter.cpp
+++ b/llvm/utils/TableGen/DXILEmitter.cpp
@@ -87,7 +87,7 @@ static void ascendingSortByVersion(std::vector<const Record *> &Recs) {
/// Take a `int_{intrinsic_name}` and return just the intrinsic_name part if
/// available. Otherwise return the empty string.
static StringRef GetIntrinsicName(const RecordVal *RV) {
- if (RV && RV->getValue()) {
+ if (RV) {
if (const DefInit *DI = dyn_cast<DefInit>(RV->getValue())) {
auto *IntrinsicDef = DI->getDef();
auto DefName = IntrinsicDef->getName();
diff --git a/llvm/utils/TableGen/RegisterInfoEmitter.cpp b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
index 05a7ff8b77c2d7..4e2a6abcaacfc8 100644
--- a/llvm/utils/TableGen/RegisterInfoEmitter.cpp
+++ b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
@@ -505,7 +505,7 @@ void RegisterInfoEmitter::EmitRegMappingTables(
for (auto &RE : Regs) {
const Record *Reg = RE.TheDef;
const RecordVal *V = Reg->getValue("DwarfAlias");
- if (!V || !V->getValue())
+ if (!V)
continue;
const DefInit *DI = cast<DefInit>(V->getValue());
diff --git a/mlir/lib/TableGen/Attribute.cpp b/mlir/lib/TableGen/Attribute.cpp
index ec7f4faafeb4e1..f31644b772cf35 100644
--- a/mlir/lib/TableGen/Attribute.cpp
+++ b/mlir/lib/TableGen/Attribute.cpp
@@ -125,7 +125,7 @@ StringRef Attribute::getDerivedCodeBody() const {
Dialect Attribute::getDialect() const {
const llvm::RecordVal *record = def->getValue("dialect");
- if (record && record->getValue()) {
+ if (record) {
if (const DefInit *init = dyn_cast<DefInit>(record->getValue()))
return Dialect(init->getDef());
}
diff --git a/mlir/lib/TableGen/Builder.cpp b/mlir/lib/TableGen/Builder.cpp
index a94e1cca5fc59e..672395367de320 100644
--- a/mlir/lib/TableGen/Builder.cpp
+++ b/mlir/lib/TableGen/Builder.cpp
@@ -30,7 +30,7 @@ StringRef Builder::Parameter::getCppType() const {
// Inlining the first part of `Record::getValueAsString` to give better
// error messages.
const llvm::RecordVal *type = record->getValue("type");
- if (!type || !type->getValue()) {
+ if (!type) {
llvm::PrintFatalError("Builder DAG arguments must be either strings or "
"defs which inherit from CArg");
}
diff --git a/mlir/lib/TableGen/Property.cpp b/mlir/lib/TableGen/Property.cpp
index 88a64891b8b868..d9edbe23d453fa 100644
--- a/mlir/lib/TableGen/Property.cpp
+++ b/mlir/lib/TableGen/Property.cpp
@@ -99,7 +99,7 @@ Pred Property::getPredicate() const {
if (!def)
return Pred();
const llvm::RecordVal *maybePred = def->getValue("predicate");
- if (!maybePred || !maybePred->getValue())
+ if (!maybePred)
return Pred();
return Pred(maybePred->getValue());
}
diff --git a/mlir/lib/TableGen/Type.cpp b/mlir/lib/TableGen/Type.cpp
index 39d6e64c91d095..4d2cedbff66e99 100644
--- a/mlir/lib/TableGen/Type.cpp
+++ b/mlir/lib/TableGen/Type.cpp
@@ -48,7 +48,7 @@ std::optional<StringRef> TypeConstraint::getBuilderCall() const {
// Check to see if this type constraint has a builder call.
const llvm::RecordVal *builderCall = baseType->getValue("builderCall");
- if (!builderCall || !builderCall->getValue())
+ if (!builderCall)
return std::nullopt;
return TypeSwitch<const llvm::Init *, std::optional<StringRef>>(
builderCall->getValue())
diff --git a/mlir/lib/Tools/tblgen-lsp-server/TableGenServer.cpp b/mlir/lib/Tools/tblgen-lsp-server/TableGenServer.cpp
index 2dae2448fda306..71b6dfed373c0b 100644
--- a/mlir/lib/Tools/tblgen-lsp-server/TableGenServer.cpp
+++ b/mlir/lib/Tools/tblgen-lsp-server/TableGenServer.cpp
@@ -581,7 +581,7 @@ TableGenTextFile::buildHoverForRecord(const Record *record,
// Check that the record actually has the given field, and that it's a
// string.
const RecordVal *value = record->getValue(fieldName);
- if (!value || !value->getValue())
+ if (!value)
return;
auto *stringValue = dyn_cast<llvm::StringInit>(value->getValue());
if (!stringValue)
More information about the Mlir-commits
mailing list