<div dir="ltr">Sure, that should work just fine. Could you also add a comment what events setting the log file sets in motion? It is not obvious from the test code.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 11, 2019 at 6:51 PM Alex L <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hmm, the logging was meant to exercise the creation of chained diagnostic consumer, so without it the test is kind of pointless. I'll undo your change, but will set the file to "-" which will print the log to STDERR and won't create new files. Does that sound reasonable?<div><br></div><div>Cheers,</div><div>Alex </div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 11 Jun 2019 at 02:51, Ilya Biryukov <<a href="mailto:ibiryukov@google.com" target="_blank">ibiryukov@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Alex,<div><br></div><div>Just wanted to let you know that I removed logging of diagnostics into a file inside the unit test in r363041 to unbreak our integrate.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 11, 2019 at 1:29 AM Alex Lorenz via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: arphaman<br>
Date: Mon Jun 10 16:32:42 2019<br>
New Revision: 363009<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=363009&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=363009&view=rev</a><br>
Log:<br>
[Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer<br>
in the compiler<br>
<br>
The function SetUpDiagnosticLog that was called from createDiagnostics didn't<br>
handle the case where the diagnostics engine didn't own the diagnostics consumer.<br>
This is a potential problem for a clang tool, in particular some of the follow-up<br>
patches for clang-scan-deps will need this fix.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D63101" rel="noreferrer" target="_blank">https://reviews.llvm.org/D63101</a><br>
<br>
Modified:<br>
cfe/trunk/lib/Frontend/CompilerInstance.cpp<br>
cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp<br>
<br>
Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=363009&r1=363008&r2=363009&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=363009&r1=363008&r2=363009&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Jun 10 16:32:42 2019<br>
@@ -232,9 +232,13 @@ static void SetUpDiagnosticLog(Diagnosti<br>
std::move(StreamOwner));<br>
if (CodeGenOpts)<br>
Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags);<br>
- assert(Diags.ownsClient());<br>
- Diags.setClient(<br>
- new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));<br>
+ if (Diags.ownsClient()) {<br>
+ Diags.setClient(<br>
+ new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));<br>
+ } else {<br>
+ Diags.setClient(<br>
+ new ChainedDiagnosticConsumer(Diags.getClient(), std::move(Logger)));<br>
+ }<br>
}<br>
<br>
static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts,<br>
<br>
Modified: cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp?rev=363009&r1=363008&r2=363009&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp?rev=363009&r1=363008&r2=363009&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp (original)<br>
+++ cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp Mon Jun 10 16:32:42 2019<br>
@@ -8,6 +8,7 @@<br>
<br>
#include "clang/Frontend/CompilerInstance.h"<br>
#include "clang/Frontend/CompilerInvocation.h"<br>
+#include "clang/Frontend/TextDiagnosticPrinter.h"<br>
#include "llvm/Support/FileSystem.h"<br>
#include "llvm/Support/Format.h"<br>
#include "llvm/Support/ToolOutputFile.h"<br>
@@ -70,4 +71,21 @@ TEST(CompilerInstance, DefaultVFSOverlay<br>
ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file"));<br>
}<br>
<br>
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {<br>
+ auto DiagOpts = new DiagnosticOptions();<br>
+ DiagOpts->DiagnosticLogFile = "log.diags";<br>
+<br>
+ // Create the diagnostic engine with unowned consumer.<br>
+ std::string DiagnosticOutput;<br>
+ llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);<br>
+ auto DiagPrinter = llvm::make_unique<TextDiagnosticPrinter>(<br>
+ DiagnosticsOS, new DiagnosticOptions());<br>
+ CompilerInstance Instance;<br>
+ IntrusiveRefCntPtr<DiagnosticsEngine> Diags = Instance.createDiagnostics(<br>
+ DiagOpts, DiagPrinter.get(), /*ShouldOwnClient=*/false);<br>
+<br>
+ Diags->Report(diag::err_expected) << "no crash";<br>
+ ASSERT_EQ(DiagnosticsOS.str(), "error: expected no crash\n");<br>
+}<br>
+<br>
} // anonymous namespace<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail-m_-5301323491383978306gmail-m_5391524770235623077gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>
</blockquote></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>