Thanks, that sounds good to me. I added the assignment with the new comment in 363199.
On Wed, 12 Jun 2019 at 03:59, Ilya Biryukov <ibiryu...@google.com> wrote: > 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
