[PATCH] D47535: [FileSpec] Add support for lambdas to EnumerateDirectory. NFC

[Pavel Labath via Phabricator via llvm-commits]

labath added a reviewer: zturner.
labath added a comment.

In https://reviews.llvm.org/D47535#1116392, @JDevlieghere wrote:

> In https://reviews.llvm.org/D47535#1116364, @labath wrote:
>
> > Actually, I wonder if we shouldn't just deprecate this function altogether. What was your motivation for this patch? It seems we already have `llvm::fs::(recursive_)directory_iterator` for this purpose. It would be great if we could use that for all new code. Have you looked at that?
>
>
> My motivation is https://reviews.llvm.org/D47539. I could use the iterators directly but since the FileSpec one is based on them anyway (and adds some functionality that is actually useful) I figured I might as well use them for consistency. I'm not opposed to using the iterators directly, but won't that result in more code?


Looking back at the last refactor of this function (https://reviews.llvm.org/D30807)  I think the intention even then was to deprecate/remove it altogether.

Also, I don't think that this would increase the amount of code. Looking at your patch, it seems that it could be equivalently implemented using llvm iterators as:

  std::error_code EC;
  for(llvm::sys::fs::recursive_directory_iterator Iter(dir.GetStringRef(), EC), End; Iter != End && !EC ; Iter.incement(EC)) {
    auto Status = Iter->status();
    if (!Status)
      break;
    if (llvm::sys::fs::is_regular_file(*Status) && llvm::sys::fs::can_execute(Status->path())
      executables.push_back(FileSpec(Status->path()));
  }

which is (a tiny bit) shorter. I also find code with no callbacks more readable.


Repository:
  rL LLVM

https://reviews.llvm.org/D47535