[PATCH] D130864: [NFC] Introduce ASTContext::isInSameModule()

2022-08-02 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D130864#3693022 , @ChuanqiXu wrote: > In D130864#3693019 , @ilya-biryukov > wrote: > >> Thanks for working on this. I have a few major concerns here: >> >> - Do we know that it's a bottl

[PATCH] D130864: [NFC] Introduce ASTContext::isInSameModule()

2022-08-02 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu planned changes to this revision. ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. In D130864#3693019 , @ilya-biryukov wrote: > Thanks for working on this. I have a few major concerns here: > > - Do we know that it's a bot

[PATCH] D130864: [NFC] Introduce ASTContext::isInSameModule()

2022-08-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Thanks for working on this. I have a few major concerns here: - Do we know that it's a bottleneck in practice? E.g. if the module name have different sizes, comparing strings is actually fast. If the module names are short, we are also in a pretty good situation.

[PATCH] D130864: [NFC] Introduce ASTContext::isInSameModule()

2022-08-01 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu marked 2 inline comments as done. ChuanqiXu added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:6232 + + auto getHashValueForPrimaryModule = [this](const Module *M) { +if (!PrimaryModuleHash.count(M)) erichkeane wrote: > Doesn't `FindAn

[PATCH] D130864: [NFC] Introduce ASTContext::isInSameModule()

2022-08-01 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 449173. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130864/new/ https://reviews.llvm.org/D130864 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/Sema/Sema.h clang/lib/AST/ASTCon

[PATCH] D130864: [NFC] Introduce ASTContext::isInSameModule()

2022-08-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:6217 + // If one is not module, they must not be in the same module. + if (!X != !Y) +return false; Please find another way to write this... this is about the most jarring way to co

[PATCH] D130864: [NFC] Introduce ASTContext::isInSameModule()

2022-07-31 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: iains, ilya-biryukov, erichkeane. ChuanqiXu added a project: clang-modules. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously, when