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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to