JonasToth 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;
----------------
jbcoe wrote:
> 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.
i think either is ok. maybe someone else prefers one strongly over the other,
but i dont mind.
but i think the else path should exist, make an failing assert or sth like
that, for the safety ;)
================
Comment at: clang-tools-extra/test/clang-tidy/safety-no-vector-bool.cpp:37
+std::vector<bool, user_allocator<bool>> v4;
+
----------------
jbcoe wrote:
> 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.
i found hasAnyTemplateArgument in the ast matcher refrence. did u use that one?
https://reviews.llvm.org/D29118
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
