[PATCH] D78042: Eliminate all uses of Identifier::is() in the source tree, this doesn't remove the definition of it (yet). NFC.

[Chris Lattner via Phabricator via llvm-commits]

lattner marked 4 inline comments as done.
lattner added inline comments.


================
Comment at: mlir/include/mlir/IR/Identifier.h:85
 
 // Identifier/Identifier equality comparisons are defined inline.
+inline bool operator==(Identifier lhs, StringRef rhs) {
----------------
rriddle wrote:
> nit: Can we use /// here?
This is a general comment, not a doc comment for that operator.


================
Comment at: mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp:821
     if (llvm::any_of(elidedAttrs,
-                     [&](StringRef elided) { return attr.first.is(elided); })) {
+                     [&](StringRef elided) { return attr.first == elided; })) {
       continue;
----------------
rriddle wrote:
> Do you even need the lambda anymore, i.e., could you use `llvm::is_contained` now?
I don't think that is_contained works for any of these, because it is only comparing against the first element of the pair, not against the element of the sequence.

That said, I think the real issue here is that attribute lists are being processed pervasively as ArrayRef<NamedAttributes>, which doesn't have useful helper functions.  Defining a proper type for this could make sense, but that is beyond the scope of this patch.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78042