steakhal wrote:

> This is great, thank you! I am also wondering if we want to add `getString()` 
> method to `SVal`. If we would done that then analyzers using `SVal` could 
> make a use of `Twine` by allowing a variable that has an `SVal` type to be 
> printed by using `Twine`.
> 
> As an example:
> 
> ```c++
> llvm::SmallString<128> Str;
>  llvm::raw_svector_ostream OS(Str);
> OS << " Origin " << ArgSVal << " bound to " << Region;
> auto BR = std::make_unique<PathSensitiveBugReport>(BugMsg, OS.str(), N);
> C.emitReport(std::move(BR));
> ```
> 
> Could be simply written as:
> 
> ```c++
>   auto BR = std::make_unique<PathSensitiveBugReport>(BugMsg, llvm::Twine(" 
> Origin ") + ArgSVal.getString() + " bound to " +
>       Region->getString(), N);
> ```
> 
> AFAIK this change would require to add `getString()` method to `SVal` similar 
> to how `MemRegion::getString()` works. If you agree, I'd love to work on this 
> feature.

Ideally, SVal::getString()` or the `operator<<` would not be used for reports. 
That representation is for debugging purposes only. One can acquire the VarDecl 
that a VarRegion refers to and get a proper name for it, but I think the 
framework should reserve the right for changing any of the dumps of these SVals.

Remember, most of the time, these SVals refer to things that doesn't mean 
anything for the user. Like a Conjured symbol, or a StackAddressSpaceRegion - 
thus, in general, getting the stringified SVal is a bad idea. I'd need a bit 
more convincing to expose an api that would make this mistake more tempting.

https://github.com/llvm/llvm-project/pull/205527
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to