r363009 - [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer

[Ilya Biryukov via cfe-commits]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190611/c3168075/attachment.html>