denzor200 marked an inline comment as done. denzor200 added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/boost/CMakeLists.txt:8 BoostTidyModule.cpp + UseRangeBasedForLoopCheck.cpp UseToStringCheck.cpp ---------------- LegalizeAdulthood wrote: > I am wondering if this check is better placed in the modernize module? > Maybe as an enhancement to the existing `modernize-loop-convert`? I vote against the "modernize" section for this check. See arguments above. ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst:30 +----------------- +* Client code that use ``BOOST_FOREACH`` with "``std::pair`` of iterators" or with "Null-terminated strings (``char`` and ``wchar_t``)" or some other sequence types (everything that is not declared in the list "convertible correctly" above) will produce broken code (compilation error). + ---------------- LegalizeAdulthood wrote: > Eugene.Zelenko wrote: > > Please separate with newline and follow 80 character limit. Also closing > > quote for `Null-terminated strings` is missing. > It would be better to detect known failure cases in the matcher and not issue > a fixit but perhaps issue a diagnostic. I do not understand how I can determine type of expression `list_int` (for example) at the preprocessing stage. This trick was not found in any of the existing checks. ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst:47 + +* First argument of the ``BOOST_FOREACH`` macro must be only new identifier definition, all other will produce a compilation error after migration. + ---------------- LegalizeAdulthood wrote: > Can't you detect this in the matcher and not issue a fixit in that case? Yes, i can just parse the string token `char c` into mini AST and then determine if this is a variable declaration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116577/new/ https://reviews.llvm.org/D116577 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
