r363009 - [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 12 03:59:11 PDT 2019
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 <arphaman at 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 <ibiryukov at 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 at 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 at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>>
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190612/7117fd25/attachment.html>
More information about the cfe-commits
mailing list