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

Iain Sandoe via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 27 03:35:22 PDT 2022


iains added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:621
 
+void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
+  while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())
----------------
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)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126189



More information about the cfe-commits mailing list