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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124149/new/
https://reviews.llvm.org/D124149
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
