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?
Cheers, Alex On Tue, 11 Jun 2019 at 02:51, Ilya Biryukov <ibiryu...@google.com> wrote: > Hi Alex, > > 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. > > On Tue, Jun 11, 2019 at 1:29 AM Alex Lorenz via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: arphaman >> Date: Mon Jun 10 16:32:42 2019 >> New Revision: 363009 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=363009&view=rev >> Log: >> [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer >> in the compiler >> >> The function SetUpDiagnosticLog that was called from createDiagnostics >> didn't >> handle the case where the diagnostics engine didn't own the diagnostics >> consumer. >> This is a potential problem for a clang tool, in particular some of the >> follow-up >> patches for clang-scan-deps will need this fix. >> >> Differential Revision: https://reviews.llvm.org/D63101 >> >> Modified: >> cfe/trunk/lib/Frontend/CompilerInstance.cpp >> cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp >> >> Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=363009&r1=363008&r2=363009&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) >> +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Jun 10 16:32:42 2019 >> @@ -232,9 +232,13 @@ static void SetUpDiagnosticLog(Diagnosti >> >> std::move(StreamOwner)); >> if (CodeGenOpts) >> Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags); >> - assert(Diags.ownsClient()); >> - Diags.setClient( >> - new ChainedDiagnosticConsumer(Diags.takeClient(), >> std::move(Logger))); >> + if (Diags.ownsClient()) { >> + Diags.setClient( >> + new ChainedDiagnosticConsumer(Diags.takeClient(), >> std::move(Logger))); >> + } else { >> + Diags.setClient( >> + new ChainedDiagnosticConsumer(Diags.getClient(), >> std::move(Logger))); >> + } >> } >> >> static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts, >> >> Modified: cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp?rev=363009&r1=363008&r2=363009&view=diff >> >> ============================================================================== >> --- cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp (original) >> +++ cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp Mon Jun 10 >> 16:32:42 2019 >> @@ -8,6 +8,7 @@ >> >> #include "clang/Frontend/CompilerInstance.h" >> #include "clang/Frontend/CompilerInvocation.h" >> +#include "clang/Frontend/TextDiagnosticPrinter.h" >> #include "llvm/Support/FileSystem.h" >> #include "llvm/Support/Format.h" >> #include "llvm/Support/ToolOutputFile.h" >> @@ -70,4 +71,21 @@ TEST(CompilerInstance, DefaultVFSOverlay >> ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file")); >> } >> >> +TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) { >> + auto DiagOpts = new DiagnosticOptions(); >> + DiagOpts->DiagnosticLogFile = "log.diags"; >> + >> + // Create the diagnostic engine with unowned consumer. >> + std::string DiagnosticOutput; >> + llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput); >> + auto DiagPrinter = llvm::make_unique<TextDiagnosticPrinter>( >> + DiagnosticsOS, new DiagnosticOptions()); >> + CompilerInstance Instance; >> + IntrusiveRefCntPtr<DiagnosticsEngine> Diags = >> Instance.createDiagnostics( >> + DiagOpts, DiagPrinter.get(), /*ShouldOwnClient=*/false); >> + >> + Diags->Report(diag::err_expected) << "no crash"; >> + ASSERT_EQ(DiagnosticsOS.str(), "error: expected no crash\n"); >> +} >> + >> } // anonymous namespace >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > > -- > Regards, > Ilya Biryukov >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
