Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | ||
---|---|---|
148 | Isn't there a readability benefit to having the parameter named here albeit unused? Or: ..., bool /*woudBeCloned*/ Do we need to drop the arg names on interface method implementations also albeit unused? The comment on wouldBeCloned above can also be dropped if we are dropping this. |
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | ||
---|---|---|
148 | I am also a bit old-fashioned that I like to see a parameter name in the *implementation* of a method (not the declaration per se), but if this is the style going forward, am okay with the change of course. |
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | ||
---|---|---|
148 | Agreed, I think dropping biases to folks using IDEs & results in an indirection when reading the code (and indeed then keeping the comment above is even less defensible as those same IDEs would provide it too and contrary to the parameter names, it can't be automatically updated). So not particularly fond of this style change. |
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | ||
---|---|---|
148 | I also prefer the variable name available than this variant. I didn't get your point in the parenthesis here Jacques, can you expand? |
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | ||
---|---|---|
148 | +1 on keeping the variable names. I'm okay with the other patch (given that there aren't that many hits in practice), though I would like clang-tidy to alert on mismatch. |
Isn't there a readability benefit to having the parameter named here albeit unused? Or:
Do we need to drop the arg names on interface method implementations also albeit unused? The comment on wouldBeCloned above can also be dropped if we are dropping this.