[PATCH] D113063: [lld-macho] Cache discovered framework paths
Jez Ng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 3 07:47:55 PDT 2021
int3 accepted this revision.
int3 added a comment.
This revision is now accepted and ready to land.
Thanks!
================
Comment at: lld/MachO/Driver.cpp:125-126
+ if (Optional<StringRef> path = resolveDylibPath(symlink.str())) {
+ resolvedFrameworks[key] = *path;
return path;
+ }
----------------
ditto, we could return the assignment value and do away with the braces
================
Comment at: lld/MachO/Driver.cpp:117-119
+ auto saved = saver.save(suffixed.str());
+ resolvedPaths[key] = saved;
+ return saved;
----------------
oontvoo wrote:
> keith wrote:
> > int3 wrote:
> > > 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
> > Thanks for the feedback! Updated accordingly.
> >
> > Is it possible for us to setup some automation here to flag these?
> > Is it possible for us to setup some automation here to flag these?
>
> I suspect it's hard because the rules can be ambiguous and sometimes opinion-based :)
> The usual "rules of thumb" I'd use is that if it's hard to tell at the callsite what the type is, then it should be spelled out. (even then there're exceptions like the iterator's types and lambda, etc)
yeah I'm not aware of a way to lint for this...
thanks for shortening the return + assignment! I was actually thinking we could go all the way and do away with `saved` too, i.e. just `return resolvedFrameworks[key] = saver.save(suffixed.str());`... then the `if` wouldn't need braces
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113063/new/
https://reviews.llvm.org/D113063
More information about the llvm-commits
mailing list