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
