[PATCH] D113063: [lld-macho] Cache discovered framework paths

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Driver.cpp:95
 
+static DenseMap<CachedHashStringRef, StringRef> resolvedPaths;
 static Optional<StringRef> findFramework(StringRef name) {
----------------
given that D113073 calls its map `resolvedLibraries`, maybe this should be `resolvedFrameworks`?


================
Comment at: lld/MachO/Driver.cpp:95
 
+static DenseMap<CachedHashStringRef, StringRef> resolvedPaths;
 static Optional<StringRef> findFramework(StringRef name) {
----------------
int3 wrote:
> given that D113073 calls its map `resolvedLibraries`, maybe this should be `resolvedFrameworks`?
also we should make sure to reset this map in `cleanupCallback` (see {D112878})


================
Comment at: lld/MachO/Driver.cpp:117-119
+          auto saved = saver.save(suffixed.str());
+          resolvedPaths[key] = saved;
+          return saved;
----------------
code style is to use explicit type names instead of `auto` unless the type is something complicated like an iterator

also I would personally prefer `return resolvedPaths[key] = second;`, but up to you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113063