[PATCH] D76903: Add a flag on the context to protect against creation of operations in unregistered dialects

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini marked 2 inline comments as done.
mehdi_amini added a comment.

In D76903#1948842 <https://reviews.llvm.org/D76903#1948842>, @bondhugula wrote:

> The commit summary opening "By default, the verifier won't allow operations with unregistered dialect. " reads as if this was the situation before.


I used the future tense here to indicate the change.

> Besides "operations with unregistered dialect", what about unregistered operations, i.e., operations that aren't part of any registered dialect?

To me “operations with unregistered dialect” is the same as “ operations that aren't part of any registered dialect”.
“Unregistered operations” on the other hand can refer to an operation with a known dialect but that isn’t registered within the dialect. There is already a per-dialect flag to control this.



================
Comment at: mlir/tools/mlir-opt/mlir-opt.cpp:89
+    "allow-unregistered-dialect",
+    cl::desc("Allow operation for unregistered dialects"), cl::init(false));
+
----------------
bondhugula wrote:
> Nit: "operation for unregistered dialects" isn't really meaningful. Rephrase here and other places.
I can’t think of anything more descriptive, please suggest an alternative if you have one


================
Comment at: mlir/tools/mlir-opt/mlir-opt.cpp:169
+                            splitInputFile, verifyDiagnostics, verifyPasses,
+                            allowUnregisteredDialects));
 }
----------------
bondhugula wrote:
> `clAllowUnregisteredDialects` ?
Is this a documented or widespread convention?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76903/new/

https://reviews.llvm.org/D76903