jbcoe added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/safety/NoVectorBoolCheck.cpp:50
+ diag(MatchedDecl->getLocation(),
+ " function %0 returns an instance of std::vector<bool>")
+ << MatchedDecl;
----------------
djehuti wrote:
> JonasToth wrote:
> > i think all those diag() calls can be merged into one. inside the
> > if/else-if you can just set a StringRef with the specific part of the
> > warning, and have a parameterized diag() at the end of the function.
> >
> > in NoMallocCheck there is a similar pattern:
> >
> > const CallExpr *Call = nullptr;
> >
> > StringRef Recommendation;
> >
> >
> >
> > if ((Call = Result.Nodes.getNodeAs<CallExpr>("aquisition")))
> >
> > Recommendation = "consider a container or a smart pointer";
> >
> > else if ((Call = Result.Nodes.getNodeAs<CallExpr>("realloc")))
> >
> > Recommendation = "consider std::vector or std::string";
> >
> > else if ((Call = Result.Nodes.getNodeAs<CallExpr>("free")))
> >
> > Recommendation = "use RAII";
> >
> >
> >
> > assert(Call && "Unhandled binding in the Matcher");
> >
> >
> >
> > diag(Call->getLocStart(), "do not manage memory manually; %0")
> >
> > << Recommendation << SourceRange(Call->getLocStart(),
> > Call->getLocEnd());
> >
> Except with braces, right? (That's another High-Integrity C++ rule btw.) ;)
I agree that this _can_ be done but I'm not convinced it helps readability.
Repetition is partial and very localized. I'll happily make the change if you
feel strongly that it's an improvement.
================
Comment at: clang-tools-extra/test/clang-tidy/safety-no-vector-bool.cpp:37
+std::vector<bool, user_allocator<bool>> v4;
+
----------------
JonasToth wrote:
> what happens for types where std::vector<bool> would be an template argument?
> for example std::pair and tuple could contain a vector<bool>.
> is there a warning as well?
Nicely spotted. Those won't get picked up right now and need to be.
I'm struggling to build a matcher for this. We really need to find any place
where `std::vector<bool>` is used as a template argument.
https://reviews.llvm.org/D29118
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
