rsmith added a comment.

Thanks, this looks functionally good to me. I'm happy for this to land with the 
`Sema` member renamed.



================
Comment at: clang/lib/Sema/SemaModule.cpp:727
+    ModuleMap &Map = PP.getHeaderSearchInfo().getModuleMap();
+    GlobalModuleFragmentCache = Map.createGlobalModuleFragmentForModuleUnit(
+        BeginLoc, getCurrentModule());
----------------
If this is supposed to be a cache, we should look for an existing global module 
fragment here rather than always creating a new one. Either this member 
shouldn't be called `...Cache` or we should do a search at this point. (I think 
we do want to support the existence of multiple different global module 
fragments in a single module, for example if we have full information about 
multiple different module interface units for the same module loaded, so I 
think just renaming the member is probably the best way to go).


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

https://reviews.llvm.org/D115610

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D115610: ... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to