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

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Wed May 30 09:34:42 PDT 2018


+1 I’d like to get rid of all EnumerateXXX with callback functions and
replace with iterator based equivalents. Given that in this case the
iterator version already exists, I definitely think we should try to use it
instead
On Wed, May 30, 2018 at 9:30 AM Pavel Labath via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180530/94927ad5/attachment-0001.html>


More information about the lldb-commits mailing list