[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

[Gabor Marton via Phabricator via cfe-commits]

martong marked 4 inline comments as done.
martong added a comment.

In D79431#2020951 <https://reviews.llvm.org/D79431#2020951>, @Szelethus wrote:

> Sure, this is an improvement because we display more information, but I'd argue that it isn't really a more readable warning message :) How about
>
> <argno>th argument to the call to <function name>
>
> - cannot be represented with a character
> - is a null pointer
> - ... , which violates the function's preconditions.
>
>   WDYT?


I'd rather prefer @balazske's approach (see below) as that is similarly expressive but sparser.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:88
   /// obviously uint32_t should be enough for all practical purposes.
   typedef uint32_t ArgNo;
   static const ArgNo Ret;
----------------
steakhal wrote:
> Shouldn't we use `using` instead?
Yes, we should. But it's legacy code from the times long before.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:323
+        std::string("Function argument constraint is not satisfied, ") +
+        VC->getName().data() + ", ArgN: " + std::to_string(VC->getArgNo());
     if (!BT_InvalidArg)
----------------
steakhal wrote:
> balazske wrote:
> > baloghadamsoftware wrote:
> > > Instead of `std::string` we usually use `llvm::SmallString` and an `llvm::raw_svector_ostream` to print into it.
> > This would look better?
> > "Function argument constraint is not satisfied, constraint: <Name>, ArgN: <ArgN>"
> Maybe `llvm::Twine` would be a better choice to `llvm::raw_svector_ostream` for string concatenations.
> docs:
> > Twine - A lightweight data structure for efficiently representing the concatenation of temporary values as strings.
We can use a Twine to create the owning std::string without creating interim objects. But then we must pass an owning object to the ctor of PathSensitiveBugReport, otherwise we might end up with dangling pointers. 
https://llvm.org/docs/ProgrammersManual.html#dss-twine


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79431