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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
