[PATCH] D87469: [LLD][ELF] Optimize linker script filename glob pattern matching NFC

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

I think this is fine approach in general. I wonder if it should/could be splitted into 2 patches though:
one for `InputSectionDescription` and one for `SectionPattern`?



================
Comment at: lld/ELF/LinkerScript.cpp:324
+static StringRef getFilename(const InputFile *file) {
+  return file ? file->getNameForScript() : StringRef();
+}
----------------
It feels that `getFilename` can instead be just inlined now, though?


================
Comment at: lld/ELF/LinkerScript.cpp:332
+  if (!matchesFileCache || matchesFileCache->first != file)
+    matchesFileCache = std::make_pair(file, filePat.match(getFilename(file)));
+
----------------
You can just call `emplace`. (And below).


================
Comment at: lld/ELF/LinkerScript.h:161
+
+  mutable llvm::Optional<std::pair<const InputFile *, bool>> excludesFileCache;
+  bool excludesFile(const InputFile *file) const;
----------------
I'd add a comment.


================
Comment at: lld/ELF/LinkerScript.h:175
 
   SingleStringMatcher filePat;
+  mutable llvm::Optional<std::pair<const InputFile *, bool>> matchesFileCache;
----------------
Should we make `InputSectionDescription` to be `class` (because it has a member function now)
and make `filePat` private?


================
Comment at: lld/ELF/LinkerScript.h:176
   SingleStringMatcher filePat;
+  mutable llvm::Optional<std::pair<const InputFile *, bool>> matchesFileCache;
+  bool matchesFile(const InputFile *file) const;
----------------
I'd add a comment here too probably.


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

https://reviews.llvm.org/D87469