[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added inline comments.


================
Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:24
+
+TextDiagnosticPrinter::TextDiagnosticPrinter(
+    raw_ostream &os, clang::DiagnosticOptions *diags)
----------------
awarzynski wrote:
> sameeranjoshi wrote:
> > A silly question from what I see usually in Flang coding style.
> > Why isn't it defined in header file?
> No such thing as silly questions! :)
> 
> AFAIK, there are no rules in the coding guidelines with regard to
> * where things should be defined, and 
> * where things should be declared. 
> I use this rather generic rule of thumb: declare in headers, define in source files. In this particular case - I followed what was already in Clang. It made sense to me.
> 
> Do you think that it would better to define this in a header file?
I think in flang the style is to declare the class in the source file if it is only used in that file. If it is used elsewhere then put it in the header. If it is used in another directory then move the header to include directory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87774