================
@@ -342,9 +342,9 @@ SarifDocumentWriter::createCodeFlow(ArrayRef<ThreadFlow> 
ThreadFlows) {
   return json::Object{{"threadFlows", createThreadFlows(ThreadFlows)}};
 }
 
-void SarifDocumentWriter::createRun(StringRef ShortToolName,
-                                    StringRef LongToolName,
-                                    StringRef ToolVersion) {
+void SarifDocumentWriter::createRun(std::string ShortToolName,
+                                    std::string LongToolName,
+                                    std::string ToolVersion) {
----------------
steakhal wrote:

I'm not convinced by this argument.

In this function `ShortToolName` and friends are passed to the `json::Object` 
that will internally store them as `std::string` (aka. copy into a new 
instance, so no risk of anything getting dangling).

And since we are talking about function parameters, the arguments doens't 
matter if they are temporaries or not. They are alive until the call returns - 
so there is no dangling risk either.

As a rule of thumb, functions should take view/ref types, unless they "consume" 
the object (which is not the case here - there are no std::moves here). This is 
what caught my attention, and still doesn't quite make sense.

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

Reply via email to