[PATCH] D113715: [mlir][ods] AttrOrTypeGen uses Class

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/include/mlir/TableGen/Class.h:436-439
+inline constexpr mlir::tblgen::Method::Properties
+operator&(mlir::tblgen::Method::Properties lhs, bool rhs) {
+  return rhs ? lhs : mlir::tblgen::Method::None;
+}
----------------
Please remove this, the code that this creates is not readable.


================
Comment at: mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp:1950-1951
 
-  auto properties = method.isStatic() ? Method::MP_Static : Method::MP_None;
-  if (declaration)
-    properties =
-        static_cast<Method::Property>(properties | Method::MP_Declaration);
-  return opClass.addMethod(method.getReturnType(), method.getName(), properties,
+  auto props = (Method::Static & method.isStatic()) |
+               (Method::Declaration & declaration);
+  return opClass.addMethod(method.getReturnType(), method.getName(), props,
----------------
rriddle wrote:
> Mogball wrote:
> > rriddle wrote:
> > > Is this right, AFAICT `Method::Declaration & declaration` is always false.
> > This actually expands to `declaration ? Method::Declaration : Method::None`. If that's not intuitive, I can remove it.
> Are you implying `Method::Declaration & declaration` is the same as `declaration ? Method::Declaration : Method::None`?
> 
> https://godbolt.org/z/rnjhbe77G
I see now, you've defined a custom operator& for that. Please remove that, it creates a hacky difference in behavior that is not what someone what naturally expect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113715