[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

Sun Jul 23 08:55:13 PDT 2023

PiotrZSL added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:83
   Diags.setSourceManager(&Sources);
+  ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts());

----------------
carlosgalvezp wrote:
> A bit unclear to me why we should add this line here, grepping for this function in the repo I only find hits in the `clang` folder. How come it's not needed in other places?
We create here new Preprocessor (line 96) and new DiagEngine (line 74), when C++20/Modules are enabled this class is register as an second Preprocessor and both are (+-) executed.
Unfortunately when we pass `-Wno-macro-redefined` it's pass only to original DiagEngine, and we run into situation when warning is suppressed by first DiagEngine, but not by second that is used by second Preprocessor. 

Passing DiagnosticOptions alone to DiagEngine looks to be insufficient, as it's does not apply settings, only calling this function apply them. (somehow).
This is gray area for me.

More about problem here: https://discourse.llvm.org/t/rfc-expand-modular-headers-ppcallbacks-problem-in-c-20/71628

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156056