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.
On Tue, Jun 11, 2019 at 6:51 PM Alex L <arpha...@gmail.com> wrote: > 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 >> > -- Regards, Ilya Biryukov
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
