[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:621
 
+void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
+  while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
----------------
iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > ChuanqiXu wrote:
> > > > Recommend to comment related part in your summary. It should be much helpful for other to read the codes.
> > > Hmm. I am not sure exactly what you mean here - there is a comment about the purpose of the method, where the method is declared.
> > > The commit message describes what this method does in the first part.  I'm happy to make things more clear, but not sure where you want to see some change,
> > I mean the paragraph:
> > ```
> > For a module (instead of the generic CXX initializer) we emit a module init
> > that:
> > 
> > - wraps the contained initializations in a control variable to ensure that the inits only happen once, even if a module is imported many times by imports of the main unit.
> > - calls module initialisers for imported modules first. Note that the order of module import is not significant, and therefore neither is the order of imported module initializers.
> > - We then call initializers for the Global Module Fragment (if present)
> > - We then call initializers for the current module.
> > - We then call initializers for the Private Module Fragment (if present)
> > ```
> > 
> > I understand people might feel like this is wordy. But, **personally**, I prefer readability.
> so, to clarify - you would like me to add this description as a comment block on the new initializer method (I am OK with doing this)
Yeah, I feel it is helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126189