Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/IR/Identifier.h | ||
---|---|---|
85–86 | nit: Can we use /// here? | |
mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp | ||
830 | Do you even need the lambda anymore, i.e., could you use llvm::is_contained now? | |
898 | Same here. | |
mlir/test/lib/Dialect/Test/TestDialect.cpp | ||
411 | Same here. | |
mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp | ||
619 | Same here |
mlir/include/mlir/IR/Identifier.h | ||
---|---|---|
85–86 | This is a general comment, not a doc comment for that operator. | |
mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp | ||
830 | 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. |
nit: Can we use /// here?