[PATCH] D113676: WIP: [clang][lex] Fix search path usage remark with modules

Alex Hoppen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 12 02:44:28 PST 2021

ahoppen added a comment.

LGTM overall. I’ve got a few questions/remarks inline.

Comment at: clang/lib/Lex/HeaderSearch.cpp:94
+      // Module map parsing initiated by header search.
+      if (HS.CurrentSearchPathIdx != ~0U)
+        HS.ModuleToSearchDirIdx[M] = HS.CurrentSearchPathIdx;
When would the `moduleMapModuleCreated` be called while `CurrentSearchPathIdx == ~0U`? Could this be an `assert` instead?

Comment at: clang/lib/Lex/HeaderSearch.cpp:264
+  if (Module || !AllowSearch || !HSOpts->ImplicitModuleMaps) {
+    noteModuleLookupUsage(Module, ImportLoc);
     return Module;
Just a thought: Could we move `noteModuleLookupUsage` into `findModule`? IIUC that would guarantee that we’re catching all cases and we wouldn’t need to call `noteModuleLookupUsage ` from both overloads of `lookupModule`.

Comment at: clang/lib/Lex/ModuleMap.cpp:851
       new Module("<private>", Loc, Parent, /*IsFramework*/ false,
                  /*IsExplicit*/ true, NumCreatedModules++);
   Result->Kind = Module::PrivateModuleFragment;
Maybe a stupid question but why don’t we need to call the callback e.g. here (or on the other `new Module`) calls in this file?

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list