[clang] [clang] [diagnostics] Add `-fdiagnostics-add-output` switch for SARIF (PR #185201)

[Balázs Benics via cfe-commits]

================
@@ -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