Harald-R wrote: > Can we use this option? > https://clang.llvm.org/extra/clang-tidy/checks/bugprone/use-after-move.html#cmdoption-arg-InvalidationFunctions > I think you can place there `mlir::Operation::erase` and other friends - > this should work out of the box.
This is interesting. I did a few experiments to see how this works and I see the following behavior when setting `InvalidationFunctions` to `::mlir::Operation::erase;::mlir::RewriterBase::replaceOp`. The following cases are detected: ```cpp mlir::Operation *op; op->erase(); op->dump(); mlir::func::FuncOp myFuncOp; // FuncOp is an example of a derived operation; internally, it wraps an mlir::Operation* auto op = myFuncOp.getOperation(); // Obtain underlying mlir::Operation* op->erase(); op->dump(); ``` However, the following cases are not detected: ```cpp mlir::func::FuncOp myFuncOp; myFuncOp->erase(); // Calls erase() on the underlying mlir::Operation* myFuncOp->dump(); mlir::Operation *op; mlir::RewriterBase rewriter(op); rewriter.replaceOp(op, op); op->dump(); ``` The first one is specifically detected by `llvm-mlir-use-after-erase` as it checks for `mlir::OpState`, which is the parent class that wraps `mlir::Operation*` for all derived operations. The second one is not detected by `bugprone-use-after-move` because it assumes that the first argument of the given member function is always `this`; in this case, `mlir::RewriterBase::replaceOp` receives the argument to erase explicitly. It should be possible to generalize `InvalidationFunctions` so that the first argument assumption is avoided. Not sure however about the limitation around derived operations. It looks like we have two ways forward: refactor `bugprone-use-after-move` so that it is able to detect the remaining scenarios and have each user specifically configure all relevant MLIR methods when using this check, or introduce `llvm-mlir-use-after-erase` which has all the relevant methods pre-configured. For convenience as a user, I lean towards a dedicated check. However, there might be some disadvantages to having a dedicated check that I am not familiar with. The [duplication](https://github.com/llvm/llvm-project/pull/210727#discussion_r3645158314) of the finder class can be avoided. @vbvictor what do you think? https://github.com/llvm/llvm-project/pull/210727 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
