[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 17 22:32:47 PST 2022

xazax.hun added a comment.


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?

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list