[Lldb-commits] [clang] [clang-tools-extra] [lldb] [clang] Remove intrusive reference count from `DiagnosticOptions` (PR #139584)

[Jan Svoboda via lldb-commits]

jansvoboda11 wrote:

Thanks for the concrete example! I think the key thing to realize is that the `DiagnosticsEngine` and `DiagnosticOptions` used for command-line parsing are typically throwaway and separate from those used for actual compilation. I suggest looking at how `cc1_main()` orchestrates this and why. The intent is for the `DiagnosticsEngine` used during the compilation (owned by `CompilerInstance`) to refer to the fully-formed `DiagnosticsOptions` (owned by `CompilerInvocation`).

So concretely, I'd rewrite your example as:

```c++
  clang::DiagnosticOptions diagnostic_options;
  llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine> diagnostics_engine =
      clang::CompilerInstance::createDiagnostics(
          *file_system, diagnostic_options,
          new clang::TextDiagnosticPrinter(llvm::errs(), diagnostic_options));
  clang::CreateInvocationOptions ci_opts;
  ci_opts.Diags = diagnostics_engine;
  // ...
  std::shared_ptr<clang::CompilerInvocation> invocation =
      clang::createInvocation(args, ci_opts);

  auto compiler_instance =
      std::make_unique<clang::CompilerInstance>(invocation);
  compiler_instance->createDiagnostics(*file_system));
  // ...
  return compiler_instance;
```

I agree the lifetimes shouldn't be this complicated. FWIW I'd like to get rid of the reference count of `DiagnosticsEngine` too, and make the lifetimes stricter and more explicit, but that's a lower priority compared to the `DiagnosticOptions` refactor for me.

https://github.com/llvm/llvm-project/pull/139584