NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land.
F17741480: 704.jpg <https://reviews.llvm.org/F17741480> I only have a few nits left. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:35 #include <string> +#include <variant> ---------------- I think you're not using this anymore. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:316 + +class OperatorKind { + union { ---------------- One good place to put this may be `CheckerHelpers.h`. This is where we dump all the stuff that's probably useful in multiple checkers but not in other places. I also wonder if you plan to support unary operators. The interesting part about them is that they are sometimes ambiguous to binary operators in their string representation, eg. `-`. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:435 + operationKindFromOverloadedOperator(OOK).GetBinaryOpUnsafe(); + auto RetVal = C.getSValBuilder().evalBinOp( + State, BOK, FirstPtrVal, SecondPtrVal, Call.getResultType()); ---------------- `C.getSValBuilder()` is called three times now, you might want to stash it in a reference. There's probably not much performance benefit but the code should be easier to read this way. ================ Comment at: clang/test/Analysis/Inputs/system-header-simulator-cxx.h:983 +template <typename T1, typename T2> +bool operator==(const unique_ptr<T1> &x, const unique_ptr<T2> &y); + ---------------- I think it's a good idea to test these cross-type comparison operators as well. IIUC they're handled exactly the same as their same-type counterparts but it may uncover occasional assertion failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104616/new/ https://reviews.llvm.org/D104616 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
