[PATCH] D124149: [NFC] follow up code cleanup after D123837

Chuanqi Xu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 20 23:40:53 PDT 2022

ChuanqiXu added inline comments.

Comment at: clang/lib/Sema/SemaLookup.cpp:1573
   //   attached to the global module and usable within the module unit.
-  if ((M->isGlobalModule() && !M->Parent) ||
-      // If M is the private module fragment, it is usable only if it is within
-      // the current module unit. And it must be the current parsing module unit
-      // if it is within the current module unit according to the grammar of the
-      // private module fragment.
-      // NOTE: This is covered by the following condition. The intention of the
-      // check is to avoid string comparison as much as possible.
-      (M->isPrivateModule() && M == getCurrentModule()) ||
+  if (M == GlobalModuleFragment ||
+      // If M is the module we're parsing, it should be usable. This covers the
The definition of `GlobalModuleFragment` is `The global module fragment of the current translation unit.` So I feel it more natural to use this and I don't need to judge if it is GlobalModuleFragment anymore. The original looks more hack a little bit.

Comment at: clang/lib/Sema/SemaLookup.cpp:1581
+      // comparison as much as possible.
+      M == getCurrentModule() ||
       // The module unit which is in the same module with the current module
And we don't need to check module private specially. So that we could filter more cases to avoid string comparison.

Comment at: clang/lib/Sema/SemaLookup.cpp:1784-1790
+  // means it is part of the current module.
+  if (ModulePrivate && isUsableModule(M))
+    return true;
+  // For a query which is not module-private, that means it is in our visible
+  // module set.
+  if (!ModulePrivate && VisibleModules.isVisible(M))
LLVM prefer shorter indentation. (Although I repeat `ModulePrivate` here, I feel the current one is better)

Comment at: clang/lib/Sema/SemaLookup.cpp:1787-1804
-    if (isUsableModule(M))
-      return true;
-    else if (M->Kind == Module::ModuleKind::ModulePartitionImplementation &&
-             isModuleDirectlyImported(M))
-      // Unless a partition implementation is directly imported it is not
-      // counted as visible for lookup, although the contained decls might
-      // still be reachable.  It's a partition, so it must be part of the
After D123837 is landed, the following two checks wouldn't be hit any more.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list