[PATCH] D52076: [CodeComplete] Add completions for filenames in #include directives.

[Sam McCall via Phabricator via cfe-commits]

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