LegalizeAdulthood added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp:68
+ ColToken->getLocation()),
+ (llvm::Twine("for(") + VariableString + " : ").str());
+ }
----------------
Now that this is chained, you might need to `clang-format` on it again.
Probably best to just `clang-format` the whole file before you submit.
================
Comment at: clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h:28
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus11;
+ }
----------------
This is an implicit conversion from `unsigned` to `bool`,
an explicit comparison against `!= 0` would be preferable IMO.
However, my bigger question is what happens when someone
runs this check with `-std=c++14` or some other later version of C++?
I looked at the header for `LangOptions` and it isn't clear whether
the bits are set to indicate supported portions of the standard or
if they simply reflect the command-line options used when invoking
the compiler.
================
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).
+
----------------
denzor200 wrote:
> 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.
Ah, yes, ugh. I forgot that this is all done by looking at macros.
We really need a bit of infrastructure in clang-tidy to associate macro
expansions with AST nodes for preprocessor related checks.
I suppose you could match for loops and try to correlate them to
the BOOST_FOREACH macros. Then you could reason more about
the arguments to BOOST_FOREACH via the AST, but this is probably
fragile because it depends on the implementation of BOOST_FOREACH,
which may change.
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
