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