danielmarjamaki added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:211
       // Generate a report for this bug.
-      StringRef Desc =
-          describeUninitializedArgumentInCall(Call, IsFirstArgument);
+      std::string Desc =
+          describeUninitializedArgumentInCall(Call, ArgumentNumber);
----------------
zaks.anna wrote:
> danielmarjamaki wrote:
> > zaks.anna wrote:
> > > Have you considered using  llvm::raw_svector_ostream here as well as 
> > > passing it an argument to describeUninitializedArgumentInCall? For 
> > > example, see  MallocChecker.cpp.
> > I changed so describeUninitializedArgumentInCall() returns an llvm::Twine 
> > instead of std::string. hope you like it.
> > 
> I do not think it's safe to use llvm:Twine here. See 
> http://llvm.org/docs/ProgrammersManual.html#the-twine-class
> 
> How about using llvm::raw_svector_ostream as I suggested?
sure I can use llvm::raw_svector_ostream instead. I can try to update the patch 
soon.

I just wonder how it is unsafe. I did consider if llvm::Twine would be safe. Is 
there a particular return that you can point out? The function mostly returns 
constant string literals. Those should be safe right? Then the function also 
have a few returns like this:
```
    return llvm::Twine(ArgumentNumber + 1) +
               llvm::getOrdinalSuffix(ArgumentNumber + 1) +
               " function call argument is an uninitialized value";
```
Yes we need to be careful for such code. However since llvm::getOrdinalSuffix() 
returns a StringRef this particular code should be safe right?

When the stream is used I have to tweak each return statement. I thought that 
was a bit unfortunate. But it's not a biggie.



Repository:
  rL LLVM

https://reviews.llvm.org/D30341



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to