Skip to content

Commit c040172

Browse files
[mlir][TableGen] Verify compatibility of tblgen::Method properties (#147979)
Following a recent discovery of a method being defined both "inline" and "declaration" (declaration implying no definition), verify the method properties in general to fail early in the development and avoid accidental bugs (especially for "opt-in" features).
1 parent ea65415 commit c040172

File tree

2 files changed

+48
-2
lines changed

2 files changed

+48
-2
lines changed

mlir/include/mlir/TableGen/Class.h

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,13 +332,23 @@ class Method : public ClassDeclarationBase<ClassDeclaration::Method> {
332332
: properties(properties),
333333
methodSignature(std::forward<RetTypeT>(retType),
334334
std::forward<NameT>(name), std::forward<Args>(args)...),
335-
methodBody(properties & Declaration) {}
335+
methodBody(properties & Declaration) {
336+
if (!methodPropertiesAreCompatible(properties)) {
337+
llvm::report_fatal_error(
338+
"Invalid combination of method properties specified");
339+
}
340+
}
336341
/// Create a method with a return type, a name, method properties, and a list
337342
/// of parameters.
338343
Method(StringRef retType, StringRef name, Properties properties,
339344
std::initializer_list<MethodParameter> params)
340345
: properties(properties), methodSignature(retType, name, params),
341-
methodBody(properties & Declaration) {}
346+
methodBody(properties & Declaration) {
347+
if (!methodPropertiesAreCompatible(properties)) {
348+
llvm::report_fatal_error(
349+
"Invalid combination of method properties specified");
350+
}
351+
}
342352

343353
// Define move constructor and assignment operator to prevent copying.
344354
Method(Method &&) = default;
@@ -402,6 +412,10 @@ class Method : public ClassDeclarationBase<ClassDeclaration::Method> {
402412
MethodBody methodBody;
403413
/// Deprecation message if the method is deprecated.
404414
std::optional<std::string> deprecationMessage;
415+
416+
/// Utility method to verify method properties correctness.
417+
[[maybe_unused]] static bool
418+
methodPropertiesAreCompatible(Properties properties);
405419
};
406420

407421
/// This enum describes C++ inheritance visibility.

mlir/lib/TableGen/Class.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,38 @@ void Method::writeDefTo(raw_indented_ostream &os, StringRef namePrefix) const {
159159
os << "}\n\n";
160160
}
161161

162+
bool Method::methodPropertiesAreCompatible(Properties properties) {
163+
const bool isStatic = (properties & Method::Static);
164+
const bool isConstructor = (properties & Method::Constructor);
165+
// const bool isPrivate = (properties & Method::Private);
166+
const bool isDeclaration = (properties & Method::Declaration);
167+
const bool isInline = (properties & Method::Inline);
168+
const bool isConstexprValue = (properties & Method::ConstexprValue);
169+
const bool isConst = (properties & Method::Const);
170+
171+
// Note: assert to immediately fail and thus simplify debugging.
172+
if (isStatic && isConstructor) {
173+
assert(false && "constructor cannot be static");
174+
return false;
175+
}
176+
if (isConstructor && isConst) { // albeit constexpr is fine
177+
assert(false && "constructor cannot be const");
178+
return false;
179+
}
180+
if (isDeclaration && isInline) {
181+
assert(false &&
182+
"declaration implies no definition and thus cannot be inline");
183+
return false;
184+
}
185+
if (isDeclaration && isConstexprValue) {
186+
assert(false &&
187+
"declaration implies no definition and thus cannot be constexpr");
188+
return false;
189+
}
190+
191+
return true;
192+
}
193+
162194
//===----------------------------------------------------------------------===//
163195
// Constructor definitions
164196
//===----------------------------------------------------------------------===//

0 commit comments

Comments
 (0)