[cfe-dev] RFC: refactoring clangDriver - diagnostics classes

[David Blaikie via cfe-dev]

On Tue, Nov 24, 2020 at 7:40 AM Andrzej Warzynski via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>
> Tl;Dr;
> ------
> We would like to propose a new set of diagnostics classes for the
> clangDriver library. A proof-of-concept implementation is available
> here: https://reviews.llvm.org/D92025 (more details below).
>
> WHY
> ===
> As mentioned earlier on cfe-dev [1] [2], we are implementing the new
> Flang driver. This driver is implemented in terms of clangDriver, which
> is great in terms of code re-use across LLVM sub-projects! However, this
> also means that:
>
> * Flang depends on Clang
> * Clang contains logic that's only relevant to Flang (e.g. [3])
>
> Both of these shortcomings will go away once clangDriver becomes
> independent of Clang. In this RFC we focus on one particular dependency
> of clangDriver on Clang - *Clang's diagnostics classes*.
>
> DIAGNOSTIC CLASSES
> ==================
> clangDriver relies on Clang's powerful diagnostics classes (e.g.
> Diagnostic, DiagnosticsEngine, DiagnosticConsumer, DiagnosticBuilder)
> for its relatively basic command-line errors and warnings. These classes:
>
> * are tuned for complex frontend/language diagnostics (i.e. related to
> parsing,  semantic analysis or for clang-tidy checks)

Sorry for the slight distraction, but what's the plan for Flang's
diagnostics? Might it benefit from Clang's diagnostic infrastructure
for tracking source locations, etc?

> * cater for _many_ scenarios, e.g. there 17 overloads for `operator<<`
> for `DiagnosticBuilder` (defined in Diagnostic.h), but clangDriver only
> requires 2 (for `const char * Str` and for `StringRef`)
> * implement complex logic for tracking source locations (macro
> definition vs macro expansion, template declaration vs template
> instantiation), which are never used in clangDriver
>
> Basically, the usage of Clang's diagnostics in clangDriver is _very_
> limited.
>
> OLD APPROACH - thin abstraction layer
> =====================================
> In [2] we proposed to create a thin abstraction layer above Clang's
> diagnostics classes. That abstraction layer would satisfy the
> requirements of clangDriver, but otherwise it would be independent of
> Clang. We are now discovering that the integration of diagnostics
> classes and Clang's frontend is deeper than we originally thought. This
> integration has recently become even deeper:
>
> * https://reviews.llvm.org/D84362
>
> In my post commit review I tried highlighting why we think that that
> patch makes our original approach infeasible. We are now re-evaluating
> our original approach and would appreciate your feedback.
>
> NEW APPROACH - independent set of classes
> =========================================
> Instead, we would like to propose a new set of diagnostics classes for
> clangDriver, e.g.
>
> * DriverDiagnostic, DriverDiagnosticsEngine, DriverDiagnosticConsumer,
> DriverDiagnosticBuilder.
>
> These classes would be _much_ simplified versions of Clang's current
> diagnostic classes:
>
> * initially independent of Clang's diagnostics classes (commonalities
> will naturally emerge and eventually there will be shared infrastructure)
> * will only contain driver diagnostics, i.e. DiagnosticDriverKinds.td
> (conversely, these diagnostics will no longer be available in the
> frontend, i.e. in clangBasic)
> * only warnings and errors will be supported (no other diagnostics are
> currently generated in clangDriver)
> * no concept of source location will be supported
>
> In other words, this would be a set of trivial clones of the current
> diagnostics infrastructure.
>
> PROOF-OF-CONCEPT
> ================
> A proof-of-concept implementation of the proposed approach is available
> here: https://reviews.llvm.org/D92025. It contains:
>
> * a new library, clangDriverDiagnostics (based on diagnostics classes
> from clangBasic)
> * changes in clangDriver and the Flang driver so that diagnostics from
> clangDriverDiagnostics are used instead of those from clangBaisc
>
> It builds fine and the behaviour of the new Flang driver (flang-new)
> remains unchanged. We haven't updated clang yet and we've not tried
> trimming clangDriverDiagnostics too much. Currently it's just slightly
> simplified version of diagnostics from clangBasic. We'd need to refactor
> the contents of clang/tools/driver/ first to have a complete list of the
> required features. That's obviously going to be much tricker than the
> Flang driver. We wanted to get some initial feedback before attempting that.
>
> QUESTION
> ========
> What do you think about this approach? Is there anything that we missed?
> Your comments and feedback are much appreciated!
>
> Thank you for reading!
>
> On behalf of the Arm Fortran Team,
> -Andrzej
>
> [1] http://lists.llvm.org/pipermail/llvm-dev/2020-June/141994.html
> [2] http://lists.llvm.org/pipermail/cfe-dev/2020-July/066393.html
> [3]
> https://github.com/llvm/llvm-project/blob/006b3bdeddb0234f53a1ab72e427ef74184461f5/clang/lib/Driver/Driver.cpp#L216
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev