JDevlieghere added a comment. In https://reviews.llvm.org/D47535#1116430, @labath wrote:
> 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. Fair enough, I'll update the patch :-) Repository: rL LLVM https://reviews.llvm.org/D47535 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
