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

[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 compare these...

perhaps you can take advantage of the fact that the compare on 6222 basically does the other half to this:

`if (!X || !Y)`

replaces the next compare as well.  


================
Comment at: clang/lib/AST/ASTContext.cpp:6232
+
+  auto getHashValueForPrimaryModule = [this](const Module *M) {
+    if (!PrimaryModuleHash.count(M))
----------------
Doesn't `FindAndConstruct` do something pretty similar here?  This lambda is incredibly inefficient as it requires two 3 separate lookups into the map.

Instead, I believe `FindAndConstruct` will create a new entry (which can be 'set' if necessary', and then have its value returned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130864