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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to