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

Sun Mar 29 16:36:36 PDT 2020

mehdi_amini marked an inline comment as done.
mehdi_amini added a comment.

In D76903#1948937 <https://reviews.llvm.org/D76903#1948937>, @rriddle wrote:

> This is wrong. Operations are always part of a dialect. "foo" would be considered to be part of the builtin dialect, which currently has no namespace.

Do you know why I see this?

  echo '"foo"() : () -> ()' | ./bin/mlir-opt
  <stdin>:1:1: error: 'foo' op created with unregistered dialect. If this is intended, please call allowUnregisteredDialects() on the MLIRContext, or use -allow-unregistered-dialect with mlir-opt

> IMO we should set the namespace of the builtin dialect to "mlir" and the assert that operation names always have a dialect prefix.

I'd prefer this indeed.

================
Comment at: mlir/lib/Analysis/Verifier.cpp:209
+                  "MLIRContext, or use -allow-unregistered-dialect with "
+                  "mlir-opt";
+
----------------
rriddle wrote:
> nit: Should we really mention mlir-opt here? Especially given that many users have their own opt equivalents?
I usually don't try to optimize for out-of-tree users, they'll interpret however it makes sense for them (or they can patch the message if they want).
This is also a misconfiguration, not really expected to happen other than while developing, so I expect them to hit this once and fix their config.
Tests something with mlir-opt upstream however is likely much more common I believe.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76903