-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[TableGen] Change backend callback to require const RecordKeeper #111064
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-tablegen Author: Rahul Joshi (jurahul) ChangesChange TableGen backend callback function to require a const RecordKeeper argument (by changing it from function_ref to just a function pointer). This undoes parts of #104716 which was added to enable gradual migration of TableGen backends to use const RecordKeeper (by allowing either const or non-const references). Now that all backends have been migrated to const reference, we do not need this. This is a part of effort to have better const correctness in TableGen backends: Full diff: https://github.com/llvm/llvm-project/pull/111064.diff 2 Files Affected:
diff --git a/llvm/include/llvm/TableGen/TableGenBackend.h b/llvm/include/llvm/TableGen/TableGenBackend.h
index 80134179850b0e..7a6171c58749fd 100644
--- a/llvm/include/llvm/TableGen/TableGenBackend.h
+++ b/llvm/include/llvm/TableGen/TableGenBackend.h
@@ -23,8 +23,7 @@ class RecordKeeper;
class raw_ostream;
namespace TableGen::Emitter {
-// Supports const and non-const forms of callback functions.
-using FnT = function_ref<void(RecordKeeper &Records, raw_ostream &OS)>;
+using FnT = void (*)(const RecordKeeper &Records, raw_ostream &OS);
/// Creating an `Opt` object registers the command line option \p Name with
/// TableGen backend and associates the callback \p CB with that option. If
@@ -37,7 +36,9 @@ struct Opt {
/// Convienence wrapper around `Opt` that registers `EmitterClass::run` as the
/// callback.
template <class EmitterC> class OptClass : Opt {
- static void run(RecordKeeper &RK, raw_ostream &OS) { EmitterC(RK).run(OS); }
+ static void run(const RecordKeeper &RK, raw_ostream &OS) {
+ EmitterC(RK).run(OS);
+ }
public:
OptClass(StringRef Name, StringRef Desc) : Opt(Name, run, Desc) {}
@@ -45,7 +46,7 @@ template <class EmitterC> class OptClass : Opt {
/// Apply callback for any command line option registered above. Returns false
/// is no callback was applied.
-bool ApplyCallback(RecordKeeper &Records, raw_ostream &OS);
+bool ApplyCallback(const RecordKeeper &Records, raw_ostream &OS);
} // namespace TableGen::Emitter
diff --git a/llvm/lib/TableGen/TableGenBackend.cpp b/llvm/lib/TableGen/TableGenBackend.cpp
index 210fff65458627..70fccd7671e96d 100644
--- a/llvm/lib/TableGen/TableGenBackend.cpp
+++ b/llvm/lib/TableGen/TableGenBackend.cpp
@@ -25,23 +25,6 @@ using namespace TableGen::Emitter;
const size_t MAX_LINE_LEN = 80U;
-// CommandLine options of class type are not directly supported with some
-// specific exceptions like std::string which are safe to copy. In our case,
-// the `FnT` function_ref object is also safe to copy. So provide a
-// specialization of `OptionValue` for `FnT` type that stores it as a copy.
-// This is essentially similar to OptionValue<std::string> specialization for
-// strings.
-template <> struct cl::OptionValue<FnT> final : cl::OptionValueCopy<FnT> {
- OptionValue() = default;
-
- OptionValue(const FnT &V) { this->setValue(V); }
-
- OptionValue<FnT> &operator=(const FnT &V) {
- setValue(V);
- return *this;
- }
-};
-
namespace {
struct OptCreatorT {
static void *call() {
@@ -60,7 +43,7 @@ Opt::Opt(StringRef Name, FnT CB, StringRef Desc, bool ByDefault) {
/// Apply callback specified on the command line. Returns true if no callback
/// was applied.
-bool llvm::TableGen::Emitter::ApplyCallback(RecordKeeper &Records,
+bool llvm::TableGen::Emitter::ApplyCallback(const RecordKeeper &Records,
raw_ostream &OS) {
FnT Fn = CallbackFunction->getValue();
if (!Fn)
|
Change TableGen backend callback function to require a const RecordKeeper argument (by changing it from function_ref to just a function pointer). This undoes parts of llvm#104716 which was added to enable gradual migration of TableGen backends to use const RecordKeeper (by allowing either const or non-const references). Now that all backends have been migrated to const reference, we do not need this.
00ebf33
to
236b5b4
Compare
Change TableGen backend callback function to require a const RecordKeeper argument (by changing it from function_ref to just a function pointer). This undoes parts of #104716 which was added to enable gradual migration of TableGen backends to use const RecordKeeper (by allowing either const or non-const references). Now that all backends have been migrated to const reference, we do not need this.
This is a part of effort to have better const correctness in TableGen backends:
https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089