[PATCH] D103985: [lld/mac] Print dylib search details with --print-dylib-search or RC_TRACE_DYLIB_SEARCHING

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/MachO/DriverUtils.cpp:179
 
-Optional<std::string> macho::resolveDylibPath(StringRef path) {
-  // TODO: if a tbd and dylib are both present, we should check to make sure
-  // they are consistent.
-  if (fs::exists(path))
-    return std::string(path);
-  else
+static void searchedDylib(const Twine &path, bool found) {
+  if (config->printDylibSearch)
----------------
int3 wrote:
> thakis wrote:
> > int3 wrote:
> > > alexshap wrote:
> > > > thakis wrote:
> > > > > alexshap wrote:
> > > > > > perhaps, logDylibSearchResult would be a better name ?
> > > > > > also - maybe use StringRef instead of Twine ?
> > > > > The last caller in this file passes a Twine, so StringRef doesn't work.
> > > > > 
> > > > > I like the shorter name.
> > > > I think readability of the code matters, not sure if "searchedDylib" is a sufficiently descriptive name
> > > "logSearchedDylib"? I'm fine with anything, but I'd prefer not to have Java-length verbosity
> > Maybe `didSearchDylib`?
> eh, that has the same information as `searchedDylib` but is one character longer :p "log" describes what it's doing at least
Ah, naming :)

I don't love "logSearchedDylib". "log" is obviously a verb here, and it logs a "searched dylib", but what's a "searched dylib"? "searched" acts as an adjective here but isn't one and it sounds weird to me. (Then again, I'm not a native speaker – on the third hand, so are others who might read this.) "logDylibSearch" is a bit better but then it's not clear if we log the start or the end of the search. "searchedDylib" is ok since the verb is "search", it's in past tense so it's over, and "dylib" is an ok object. The drawback is that you can read "searched" as an adjective if you squint enough (cf "logSearchedDylib"), and. "didSearchDylib" addresses that.

logDylibSearchResult() feels a bit long compared to our other names.

I think the usual takeaway when you have a list of options where none is obviously the best is that the decision doesn't really matter all that much – especially if it's easy to change later on :) So I think I'll just leave this as is for now, and if anyone feels motivated enough to change it later, go for it.


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

https://reviews.llvm.org/D103985