[PATCH] D152198: [lld][COFF] Don't handle an input file multiple times when retrying

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a subscriber: mstorsjo.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: lld/COFF/Driver.cpp:259
       // winsysroot file.
       if (std::optional<StringRef> retryPath = findFile(pathStr)) {
         auto retryMb = MemoryBuffer::getFile(*retryPath, /*IsText=*/false,
----------------
aeubanks wrote:
> rnk wrote:
> > This is confusing, if the file really doesn't exist at all, won't findFile return none on the first attempt, and with your change, we will never report a file not found error? What happens, does findFile return the string without modification?
> yeah, `findFile` is weird in that it only returns null when visiting a file more than once, not if the file is not found (where it just returns the path unmodified)
Can I trouble you to take on a follow-up to fix up the naming?

I see that `findFile` has this filtering behavior: return the filename if it is new. I propose renaming it to `findFileIfNew`, and then all of the `doFind*` variants can be renamed `find*`. The same treatment has to be applied to `findLib`.

+ at mstorsjo , do you find the suggested naming better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152198