sammccall added a comment. Addressed the comments I was sure about. A couple of open questions in SemaCodeComplete about the formatting of the completions, but want to know what you think.
================ Comment at: lib/Sema/SemaCodeComplete.cpp:8046 + !EC && It != vfs::directory_iterator(); It.increment(EC)) { + if (++Count == 1000) // If we happen to hit a huge directory, + break; // bail out early so we're not too slow. ---------------- ilya-biryukov wrote: > The size of `/usr/include` on my machine is ~830 files and dirs, which is a > little close to a limit. > Not sure if the number of files grow with time or with a number of installed > packages, but maybe we want a slightly higher limit to make sure it's won > stop showing some results from this important include dir anytime soon? Wow, I only have 300. Bumped the limit to 2500. This is based on handwavy advice on the internet that bad filesystems get slow in the low thousands. ================ Comment at: lib/Sema/SemaCodeComplete.cpp:8076 + // header maps are not (currently) enumerable. + break; + case DirectoryLookup::LT_NormalDir: ---------------- ilya-biryukov wrote: > NIT: Maybe return from each branch and put llvm_unreachable at the end of the > function? > To get an error in case the new enum value is added in addition to the > compiler warning. > > Feel free to ignore if you like the current version more, the compiler > warning is not likely to go unnoticed anyway. That won't give an error, that will give undefined behavior! (If a new value is added and the warning is missed) The current code will just skip over the directory in that case, which I think is fine. (We have -Werror buildbots) (Unless you mean return a dummy value, which is a little to unusual for my taste) Repository: rC Clang https://reviews.llvm.org/D52076 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits