[PATCH] D84362: [NFC] Refactor DiagnosticBuilder and PartialDiagnostic

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D84362#2365153 <https://reviews.llvm.org/D84362#2365153>, @rjmccall wrote:

> I may not have been clear. I'm not saying SourceLocation is a meaningful concept in the driver.

Likewise, I should be more careful with respect to how I refer to `SourceLocation` (a class in Clang) and "source location" (location within a source file).

I see your point. Basically, you are suggesting that we improve the diagnostics for the driver first. And for that we'd need something like `CommandLineLocation` (a new class: location within the command invocation) that would be related to `SourceLocation` via a common base class, e.g. `AbstractLocation` (also a new class). That would indeed be a very nice thing to have.

However, this is not what we've been proposing. Our ultimate goal is a new Flang driver:

- implemented in terms of libclangDriver
- independent of Clang.

We shouldn't need to implement this extra functionality to achieve our goal. Personally I like the idea, but we need to stay focused on activities that are strictly needed for a new Flang driver. Not making things worse for Clang is an equally important goal. Improving things in Clang is something we are keen on, but are unlikely to have the bandwidth for.

It'll hard to create these thin layers above the diagnostic classes that do not depend on `SourceLocation` and `SourceManager` (and other classes that depend/require these). This is regardless of whether driver diagnostics are capable of tracking location (via e.g. `CommandLineLocation`) or not. The integration of diagnostics classes and "source location" related classes (specifically `SourceLocation` and `SourceManager`) is quite deep and this patch deepens that. IMO this patch makes things even harder for us. I'm starting to think that all this could be vastly simplified if libclangDriver had it's own diagnostics classes- a very simplified version of what's currently available in libclangBasic (again, because the driver doesn't need much). The improvements that you propose could be added at later time. Would this be acceptable? I think that the thin abstraction layer that we're discussing here would naturally emerge with time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84362/new/

https://reviews.llvm.org/D84362