tdl-g added a comment.

Thanks, all for the comments.  I believe I've addressed all comments.  Note 
that TransformerClangTidyCheck interacts awkwardly with StoreOptions; I have a 
FIXME to clean that up.



================
Comment at: 
clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:71-72
+           binaryOperator(hasOperatorName("=="),
+                          hasEitherOperand(ignoringParenImpCasts(StringNpos)),
+                          hasEitherOperand(ignoringParenImpCasts(StringFind))),
+           change(cat("!absl::StrContains(", node("string_being_searched"),
----------------
njames93 wrote:
> njames93 wrote:
> > This is dangerous, it will match on `std::string::npos == 
> > std::string::npos` and (more importantly) `strA.find(...) == 
> > strB.find(...)`. See D80054.
> Ignore me, its late.
Just to be safe, I added a test case that confirms that it doesn't match on 
those constructs.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:15
 
-   `abseil-duration-addition <abseil-duration-addition.html>`_, "Yes"
-   `abseil-duration-comparison <abseil-duration-comparison.html>`_, "Yes"
-   `abseil-duration-conversion-cast <abseil-duration-conversion-cast.html>`_, 
"Yes"
-   `abseil-duration-division <abseil-duration-division.html>`_, "Yes"
-   `abseil-duration-factory-float <abseil-duration-factory-float.html>`_, "Yes"
-   `abseil-duration-factory-scale <abseil-duration-factory-scale.html>`_, "Yes"
-   `abseil-duration-subtraction <abseil-duration-subtraction.html>`_, "Yes"
-   `abseil-duration-unnecessary-conversion 
<abseil-duration-unnecessary-conversion.html>`_, "Yes"
-   `abseil-faster-strsplit-delimiter 
<abseil-faster-strsplit-delimiter.html>`_, "Yes"
+   `abseil-duration-addition <abseil-duration-addition.html>`_,
+   `abseil-duration-comparison <abseil-duration-comparison.html>`_,
----------------
njames93 wrote:
> tdl-g wrote:
> > Eugene.Zelenko wrote:
> > > Unrelated and incorrect changes.
> > Agreed, these were generated by "clang-tidy/add_new_check.py abseil 
> > string-find-str-contains", still trying to figure out why.
> Did you invoke the python script from the clang-tidy folder or the 
> clang-tools-extra folder. The script is sensitive to the working directory it 
> was executed from, it must be executed from the clang-tidy folder otherwise 
> it fails when trying to detect auto fixes.
Possibly.  I re-ran it on a clean cloned repo from the clang-tidy folder and it 
still created some unrelated list.rst changes (though fewer than before).  So 
for now I just added the one line
I want by hand.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80023/new/

https://reviews.llvm.org/D80023



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to