[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:

Now with the `std::moves` it kindof makes sense. At least what we take by value, what we consume later.
It's just then implicitly converts to llvm::StringRef, and then the json::Object takes it as a copy internally - but at least this doesn't bother the reader when reading the callsite - unless they know what json::Object does.

Let's consider this thread resolved. I just wanted to point out I still don't think it's well motivated; but at least does what the reader would think it does. Given that taking these by value is always safe, and it's a really really cold function, I don't think we should spend more time on it.

https://github.com/llvm/llvm-project/pull/185201