xazax.hun added a comment. Hi!
Overall the check looks good to me, I have only one problem. We usually try to emit fixes if some parts of the replaced text is a macro, e.g.: #define MYMACRO(X) count(X) if (myCont.MYMACRO(X)) ... In the above code snippet, we want to avoid modifying the source inside the definition of `MYMACRO`, because that could render the code incorrect if `MYMACRO` is used in another context, e.g. with a custom container as well. I think most tidy checks are explicitly guarding the replacements to avoid these situations. Once this is fixed and a test is added, the rest looks good to me. ================ Comment at: clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp:112 + bool Negated = NegativeComparison != nullptr; + const auto *Comparison = Negated ? NegativeComparison : PositiveComparison; + ---------------- Should we have an assert to express that only one of `PositiveComparison` and `NegativeComparison` expected to be not-null? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
