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