search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.4k stars 12.15k forks source link

decouple Diagnostics from clang #49529

Open nickdesaulniers opened 3 years ago

nickdesaulniers commented 3 years ago
Bugzilla Link 50185
Version unspecified
OS All
CC @banach-space,@dwblaikie,@echristo,@jeanPerier,@kiranktp,@zygoloid,@sscalpone

Extended Description

I was surprised to find out that flang depends on clang, seemingly only for the diagnostic engine support. I can understand how the dependency came to be, but I'd imagine that such dependencies between flang and clang can accidentally grow over time.

Does it make sense to move the diagnostics engine out of clang? Should flang have it's own diagnostics engine?

banach-space commented 3 years ago

Hey Nick,

Thank you for bringing this up!

I was surprised to find out that flang depends on clang.

This dependency comes from Flang's new compiler driver, which is implemented in terms of clangDriver. This design decision was discussed in [1], [2] and [3]. We tried our best to advertise this widely (mailing lists, LLVM Weekly, Discourse), but it's tricky with multiple subprojects and different means of communication (cfe-dev, Discourse etc).

seemingly only for the diagnostic engine support

No, that's not the case. Flang only requires clangDriver, but clangDriver depends on Clang's diagnostics classes. It's a dependency that I'm finding very tricky to remove. I've tried at least a couple of times. My most recent attempt is summarised in [4]. As it's hard to identify any:

Like David has already highlighted, there are circular dependencies within Clang (e.g. diagnostics <--> AST Writer/Reader). Also, the diagnostics API is very powerful and very complex. However, clangDriver only uses a very small fraction of that. IMO, it would be better to have a dedicated, lightweight set of diagnostics classes specifically for clangDriver. As for Flang - yes, Flang's compiler driver does use Clang's DiagnosticsEngine. But that's only because clangDriver requires it.

In order to improve all this, we would have to re-organise a fair few non-trivial things in Clang. I think that that's doable and would greatly benefit the project long-term. However, it's hard without more active input from cfe-dev. I appreciate that people have other priorities and that's fair enough.

I'm more than happy to discuss this in more detail and to actively contribute to any efforts towards this.

HTH, Andrzej

[1] https://lists.llvm.org/pipermail/cfe-dev/2019-June/062669.html [2] https://lists.llvm.org/pipermail/llvm-dev/2020-June/141994.html [3] https://lists.llvm.org/pipermail/cfe-dev/2020-July/066393.html [4] https://lists.llvm.org/pipermail/cfe-dev/2020-November/067263.html

dwblaikie commented 3 years ago

(FWIW, would also be good for clang to revisit/improve some of the diagnostics design - at least last I checked it's a layering violation due to the way the diagnostics library depends on each clang library to expose the set of diagnostics - but then those clang libraries depend on the diagnostics library (creating a circular dependence - not at the linker level, but in terms of file dependencies) - likely the right structure is for the diagnostics library to be passed a set of diagnostic sets in its initialization - so then the clang driver sets up all the diagnostic sets for all the libraries it depends on and initializes the diagnostics infrastructure with that set.

& then flang could initialize it with a different set/using it as a library - that library could drop into some lower layer (LLVM itself, or something between llvm and clang/flang)

That said, I think this layering was discussed in depth when integrating flang and was chosen with all these things having been considered - you might want to skim back over the previous discussions & summarize/link them in this bug to avoid rehashing the same conversations.