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

2018-09-18 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342449: [CodeComplete] Add completions for filenames in 
#include directives. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52076?vs=165787=165903#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52076

Files:
  cfe/trunk/include/clang-c/Index.h
  cfe/trunk/include/clang/Lex/CodeCompletionHandler.h
  cfe/trunk/include/clang/Lex/Lexer.h
  cfe/trunk/include/clang/Lex/Preprocessor.h
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Frontend/ASTUnit.cpp
  cfe/trunk/lib/Lex/Lexer.cpp
  cfe/trunk/lib/Lex/Preprocessor.cpp
  cfe/trunk/lib/Parse/Parser.cpp
  cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/CodeCompletion/included-files.cpp
  cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp

Index: cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp
===
--- cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp
+++ cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp
@@ -499,6 +499,10 @@
   contexts = CXCompletionContext_NaturalLanguage;
   break;
 }
+case CodeCompletionContext::CCC_IncludedFile: {
+  contexts = CXCompletionContext_IncludedFile;
+  break;
+}
 case CodeCompletionContext::CCC_SelectorName: {
   contexts = CXCompletionContext_ObjCSelectorName;
   break;
Index: cfe/trunk/include/clang-c/Index.h
===
--- cfe/trunk/include/clang-c/Index.h
+++ cfe/trunk/include/clang-c/Index.h
@@ -5606,9 +5606,14 @@
   CXCompletionContext_NaturalLanguage = 1 << 21,
 
   /**
+   * #include file completions should be included in the results.
+   */
+  CXCompletionContext_IncludedFile = 1 << 22,
+
+  /**
* The current context is unknown, so set all contexts.
*/
-  CXCompletionContext_Unknown = ((1 << 22) - 1)
+  CXCompletionContext_Unknown = ((1 << 23) - 1)
 };
 
 /**
Index: cfe/trunk/include/clang/Parse/Parser.h
===
--- cfe/trunk/include/clang/Parse/Parser.h
+++ cfe/trunk/include/clang/Parse/Parser.h
@@ -2969,6 +2969,7 @@
   void CodeCompletePreprocessorExpression() override;
   void CodeCompleteMacroArgument(IdentifierInfo *Macro, MacroInfo *MacroInfo,
  unsigned ArgumentIndex) override;
+  void CodeCompleteIncludedFile(llvm::StringRef Dir, bool IsAngled) override;
   void CodeCompleteNaturalLanguage() override;
 };
 
Index: cfe/trunk/include/clang/Lex/Preprocessor.h
===
--- cfe/trunk/include/clang/Lex/Preprocessor.h
+++ cfe/trunk/include/clang/Lex/Preprocessor.h
@@ -1128,6 +1128,10 @@
 CodeComplete = nullptr;
   }
 
+  /// Hook used by the lexer to invoke the "included file" code
+  /// completion point.
+  void CodeCompleteIncludedFile(llvm::StringRef Dir, bool IsAngled);
+
   /// Hook used by the lexer to invoke the "natural language" code
   /// completion point.
   void CodeCompleteNaturalLanguage();
Index: cfe/trunk/include/clang/Lex/Lexer.h
===
--- cfe/trunk/include/clang/Lex/Lexer.h
+++ cfe/trunk/include/clang/Lex/Lexer.h
@@ -711,6 +711,9 @@
 
   bool isHexaLiteral(const char *Start, const LangOptions );
 
+  void codeCompleteIncludedFile(const char *PathStart,
+const char *CompletionPoint, bool IsAngled);
+
   /// Read a universal character name.
   ///
   /// \param StartPtr The position in the source buffer after the initial '\'.
Index: cfe/trunk/include/clang/Lex/CodeCompletionHandler.h
===
--- cfe/trunk/include/clang/Lex/CodeCompletionHandler.h
+++ cfe/trunk/include/clang/Lex/CodeCompletionHandler.h
@@ -14,6 +14,8 @@
 #ifndef LLVM_CLANG_LEX_CODECOMPLETIONHANDLER_H
 #define LLVM_CLANG_LEX_CODECOMPLETIONHANDLER_H
 
+#include "llvm/ADT/StringRef.h"
+
 namespace clang {
 
 class IdentifierInfo;
@@ -60,6 +62,11 @@
  MacroInfo *MacroInfo,
  unsigned ArgumentIndex) { }
 
+  /// Callback invoked when performing code completion inside the filename
+  /// part of an #include directive. (Also #import, #include_next, etc).
+  /// \p Dir is the directory relative to the include path, e.g. "a" for 
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:19:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
+// CHECK-3-NOT: foo.cc>
+// CHECK-3-NOT: foo.h>
+// CHECK-3: foosys>
+
+// Backslash handling.
+#include "a\foosys"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang 

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

2018-09-18 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342449: [CodeComplete] Add completions for filenames in 
#include directives. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52076?vs=165787=165904#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52076

Files:
  include/clang-c/Index.h
  include/clang/Lex/CodeCompletionHandler.h
  include/clang/Lex/Lexer.h
  include/clang/Lex/Preprocessor.h
  include/clang/Parse/Parser.h
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Lex/Lexer.cpp
  lib/Lex/Preprocessor.cpp
  lib/Parse/Parser.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/included-files.cpp
  tools/libclang/CIndexCodeCompletion.cpp

Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1976,6 +1976,7 @@
   case CodeCompletionContext::CCC_ObjCInstanceMessage:
   case CodeCompletionContext::CCC_ObjCClassMessage:
   case CodeCompletionContext::CCC_ObjCCategoryName:
+  case CodeCompletionContext::CCC_IncludedFile:
 // We're looking for nothing, or we're looking for names that cannot
 // be hidden.
 return;
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -1967,6 +1967,10 @@
 ArgumentIndex);
 }
 
+void Parser::CodeCompleteIncludedFile(llvm::StringRef Dir, bool IsAngled) {
+  Actions.CodeCompleteIncludedFile(Dir, IsAngled);
+}
+
 void Parser::CodeCompleteNaturalLanguage() {
   Actions.CodeCompleteNaturalLanguage();
 }
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -445,6 +445,13 @@
   return false;
 }
 
+void Preprocessor::CodeCompleteIncludedFile(llvm::StringRef Dir,
+bool IsAngled) {
+  if (CodeComplete)
+CodeComplete->CodeCompleteIncludedFile(Dir, IsAngled);
+  setCodeCompletionReached();
+}
+
 void Preprocessor::CodeCompleteNaturalLanguage() {
   if (CodeComplete)
 CodeComplete->CodeCompleteNaturalLanguage();
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -1896,6 +1896,7 @@
 /// either " or L" or u8" or u" or U".
 bool Lexer::LexStringLiteral(Token , const char *CurPtr,
  tok::TokenKind Kind) {
+  const char *AfterQuote = CurPtr;
   // Does this string contain the \0 character?
   const char *NulCharacter = nullptr;
 
@@ -1924,8 +1925,11 @@
 
 if (C == 0) {
   if (isCodeCompletionPoint(CurPtr-1)) {
-PP->CodeCompleteNaturalLanguage();
-FormTokenWithChars(Result, CurPtr-1, tok::unknown);
+if (ParsingFilename)
+  codeCompleteIncludedFile(AfterQuote, CurPtr - 1, /*IsAngled=*/false);
+else
+  PP->CodeCompleteNaturalLanguage();
+FormTokenWithChars(Result, CurPtr - 1, tok::unknown);
 cutOffLexing();
 return true;
   }
@@ -2043,16 +2047,21 @@
 if (C == '\\')
   C = getAndAdvanceChar(CurPtr, Result);
 
-if (C == '\n' || C == '\r' || // Newline.
-(C == 0 && (CurPtr-1 == BufferEnd ||  // End of file.
-isCodeCompletionPoint(CurPtr-1 {
+if (C == '\n' || C == '\r' ||// Newline.
+(C == 0 && (CurPtr - 1 == BufferEnd))) { // End of file.
   // If the filename is unterminated, then it must just be a lone <
   // character.  Return this as such.
   FormTokenWithChars(Result, AfterLessPos, tok::less);
   return true;
 }
 
 if (C == 0) {
+  if (isCodeCompletionPoint(CurPtr - 1)) {
+codeCompleteIncludedFile(AfterLessPos, CurPtr - 1, /*IsAngled=*/true);
+cutOffLexing();
+FormTokenWithChars(Result, CurPtr - 1, tok::unknown);
+return true;
+  }
   NulCharacter = CurPtr-1;
 }
 C = getAndAdvanceChar(CurPtr, Result);
@@ -2069,6 +2078,34 @@
   return true;
 }
 
+void Lexer::codeCompleteIncludedFile(const char *PathStart,
+ const char *CompletionPoint,
+ bool IsAngled) {
+  // Completion only applies to the filename, after the last slash.
+  StringRef PartialPath(PathStart, CompletionPoint - PathStart);
+  auto Slash = PartialPath.find_last_of(LangOpts.MSVCCompat ? "/\\" : "/");
+  StringRef Dir =
+  (Slash == StringRef::npos) ? "" : PartialPath.take_front(Slash);
+  const char *StartOfFilename =
+  (Slash == StringRef::npos) ? PathStart : PathStart + Slash + 1;
+  // Code completion filter range is the filename only, up to completion point.
+  

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

2018-09-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:8057
+  if (!(Filename.endswith(".h") || Filename.endswith(".hh") ||
+Filename.endswith(".H") || Filename.endswith(".hpp") ||
+Filename.endswith(".inc")))

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > Maybe do case-insensitive matching?
> > > A corner case, but still...
> > This matches Driver's behavior (I'd reuse it but I don't think Sema 
> > can/should depend on Driver). Clang doesn't think "HPP" is a valid c++ 
> > header filename.
> > I'll add a comment.
> IIUC, Driver uses this to infer the language based on file extension (when no 
> explicit -x flag was provided).
> However, this does not stop anyone from using uppercase file extensions in 
> practice (and I assume inferring on headers is also pretty rare, since all 
> compiles start with source files anyway).
> But this should rare in any case, I don't think it's a blocker.
Oh true, headers will work in practice with any extension, so Driver might be 
too restrictive.
Made these case insensitive.


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


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

2018-09-18 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lib/Sema/SemaCodeComplete.cpp:8057
+  if (!(Filename.endswith(".h") || Filename.endswith(".hh") ||
+Filename.endswith(".H") || Filename.endswith(".hpp") ||
+Filename.endswith(".inc")))

sammccall wrote:
> ilya-biryukov wrote:
> > Maybe do case-insensitive matching?
> > A corner case, but still...
> This matches Driver's behavior (I'd reuse it but I don't think Sema 
> can/should depend on Driver). Clang doesn't think "HPP" is a valid c++ header 
> filename.
> I'll add a comment.
IIUC, Driver uses this to infer the language based on file extension (when no 
explicit -x flag was provided).
However, this does not stop anyone from using uppercase file extensions in 
practice (and I assume inferring on headers is also pretty rare, since all 
compiles start with source files anyway).
But this should rare in any case, I don't think it's a blocker.


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


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

2018-09-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 165787.
sammccall marked an inline comment as done.
sammccall added a comment.

Handle completion after a backslash in MSVC-compat mode.


Repository:
  rC Clang

https://reviews.llvm.org/D52076

Files:
  include/clang-c/Index.h
  include/clang/Lex/CodeCompletionHandler.h
  include/clang/Lex/Lexer.h
  include/clang/Lex/Preprocessor.h
  include/clang/Parse/Parser.h
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Lex/Lexer.cpp
  lib/Lex/Preprocessor.cpp
  lib/Parse/Parser.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/included-files.cpp
  tools/libclang/CIndexCodeCompletion.cpp

Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -499,6 +499,10 @@
   contexts = CXCompletionContext_NaturalLanguage;
   break;
 }
+case CodeCompletionContext::CCC_IncludedFile: {
+  contexts = CXCompletionContext_IncludedFile;
+  break;
+}
 case CodeCompletionContext::CCC_SelectorName: {
   contexts = CXCompletionContext_ObjCSelectorName;
   break;
Index: test/CodeCompletion/included-files.cpp
===
--- /dev/null
+++ test/CodeCompletion/included-files.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/a
+// RUN: touch %t/foo.h && touch %t/foo.cc && touch %t/a/foosys %t/a/foosys.h
+
+// Quoted string shows header-ish files from CWD, and all from system.
+#include "foo.h"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:5:13 %t/main.cc | FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1-NOT: foo.cc"
+// CHECK-1: foo.h"
+// CHECK-1: foosys"
+
+// Quoted string with dir shows header-ish files in that subdir.
+#include "a/foosys"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:12:13 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
+// CHECK-2-NOT: foo.h"
+// CHECK-2: foosys.h"
+// CHECK-2-NOT: foosys"
+
+// Angled string showes all files, but only in system dirs.
+#include 
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:19:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
+// CHECK-3-NOT: foo.cc>
+// CHECK-3-NOT: foo.h>
+// CHECK-3: foosys>
+
+// Backslash handling.
+#include "a\foosys"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:26:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-4 %s
+// CHECK-4: foosys.h"
+
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -32,6 +32,8 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Path.h"
 #include 
 #include 
 #include 
@@ -7994,6 +7996,114 @@
   // for the expanded tokens.
 }
 
+// This handles completion inside an #include filename, e.g. #include  NativeRelDir = StringRef(RelDir);
+  llvm::sys::path::native(NativeRelDir);
+  auto FS = getSourceManager().getFileManager().getVirtualFileSystem();
+
+  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo(),
+CodeCompletionContext::CCC_IncludedFile);
+  llvm::DenseSet SeenResults; // To deduplicate results.
+
+  // Helper: adds one file or directory completion result.
+  auto AddCompletion = [&](StringRef Filename, bool IsDirectory) {
+SmallString<64> TypedChunk = Filename;
+// Directory completion is up to the slash, e.g. ' : '"');
+auto R = SeenResults.insert(TypedChunk);
+if (R.second) { // New completion
+  const char *InternedTyped = Results.getAllocator().CopyString(TypedChunk);
+  *R.first = InternedTyped; // Avoid dangling StringRef.
+  CodeCompletionBuilder Builder(CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo());
+  Builder.AddTypedTextChunk(InternedTyped);
+  // The result is a "Pattern", which is pretty opaque.
+  // We may want to include the real filename to allow smart ranking.
+  Results.AddResult(CodeCompletionResult(Builder.TakeString()));
+}
+  };
+
+  // Helper: scans IncludeDir for nice files, and adds results for each.
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem) {
+llvm::SmallString<128> Dir = IncludeDir;
+if (!NativeRelDir.empty())
+  llvm::sys::path::append(Dir, NativeRelDir);
+
+std::error_code EC;
+unsigned Count = 0;
+for (auto It = FS->dir_begin(Dir, EC);
+ !EC && It != vfs::directory_iterator(); It.increment(EC)) {
+  if (++Count == 

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

2018-09-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: lib/Lex/Lexer.cpp:2086
+  StringRef PartialPath(PathStart, CompletionPoint - PathStart);
+  auto Slash = PartialPath.rfind('/');
+  StringRef Dir =

This will probably break for backslash includes.
We should probably also handle backslashes here when `LangOpts.MSVCCompat` is 
true.


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


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

2018-09-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

After experimenting, editor support for replacing non-identifier text before 
the cursor is... spotty at best.
So switched to just replacing the last path component.

(This makes me a bit sad because the completions now have closing quotes but 
not opening ones, which looks messy)




Comment at: lib/Lex/Lexer.cpp:1931
+  StringRef PartialPath(AfterQuote, CurPtr - 1 - AfterQuote);
+  auto Slash = PartialPath.rfind('/');
+  StringRef Dir =

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > It's also valid to use `\` for path separators on Windows, maybe also 
> > > handle those?
> > > Clang also supports this when running e.g. `clang-cl /c foo.cpp`.
> > Ah yuck, that's true (for some value of "valid"). Pre-C++11 it was 
> > explicitly UB...
> > 
> > So if a codebase consistently uses backslashes, we're going to break that 
> > style at least sometimes: we can't know whether to complete `#include  > as ` > 
> > Any objection to just normalizing slashes for the whole include as part of 
> > completion?
> > e.g. `#include  > `#include `?
> > 
> > Anything else is going to be fiddly, probably only help a few users a 
> > little bit, and leave edge cases.
> If we normalize, we probably want a config option for Windows users to be 
> friendly to them.
> No good layer to add the option, though (except a compiler flag, but that 
> looks weird), so I don't think that's directly addressable. 
> 
> Also, do we want to actually change slashes **before** the path component we 
> are completing in the same include directive? 
> 1. If we do, I suspect we might have problems with the existing language 
> clients. We ran into problems when trying to add `.` to `->` fix-its.
> 2. If we don't, the completion label would look differently from what's 
> written. That's probably not a big deal.
So we can't reliably do the right thing here. e.g. consider completing a 
directory after `#include <` at the top of the file - which slash style?
I'm not sure that sometimes respecting `\`-style separators is better than 
never.
It's hard to see how we add an option for this, and unclear that it will help 
many people.

(Also this is totally nonstandard and the insertions we're suggesting will 
work. People can reasonably expect *some* friction from their tools if they're 
particular about an unusual style.)

> Also, do we want to actually change slashes before the path component we are 
> completing in the same include directive?

In the latest patch, these aren't part of the replacement range.



Comment at: lib/Sema/SemaCodeComplete.cpp:8021
+// Directory completion is up to the slash, e.g. ' : '"');
+auto R = SeenResults.insert(TypedChunk);

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > Could we avoid adding `/`, `>` and `"` if it's currently missing?
> > > Otherwise it'll be annoying for editors that auto-add closing quotes and 
> > > completions when editing existing includes, i.e.
> > > ```
> > > // It's cool if we complete bar.h" (with closing quote) here:
> > > #include "foo/^
> > > // It's annoying if we complete an extra closing quote here:
> > > #include "foo/^"
> > > ```
> > Don't editors that add quotes/parens typically avoid duplicating them when 
> > they're typed/completed?
> > 
> > This isn't actually easy to do, because after completing `file.h` you want 
> > the cursor after the closing quote, and after completing `dir/` you want 
> > the cursor inside the closing quote. I don't see a way to do this with 
> > CodeCompletionString other than generating the quote in the former case but 
> > not the latter. (At least not one that doesn't break something else...)
> > Don't editors that add quotes/parens typically avoid duplicating them when 
> > they're typed/completed?
> They do that on typing, but not on completions, which are treated literally, 
> e.g. they'll an extra quote is actually added.
> I've checked in VSCode and it produces double quotes in those cases. Not sure 
> about the other editors that have this.
> That looks like a corner case my intuition is that other editors won't do 
> what we want either.
Added logic (to the lexer) to eat the closing quote.
This makes VSCode do the right thing. Some naive editors may still get this 
wrong though.



Comment at: lib/Sema/SemaCodeComplete.cpp:8028
+CodeCompleter->getCodeCompletionTUInfo());
+  Builder.AddTextChunk(InternedPrefix);
+  Builder.AddTypedTextChunk(InternedTyped);

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > Maybe leave this out?
> > > When completing `std::^` the completion is `vector`, not `std::vector`.
> > > In the same spirit, for includes I would expect completions for `#include 
> > > ` to be `bar.h`, not ``.
> > I tried a few iterations here and this was the only one that 

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

2018-09-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 165759.
sammccall added a comment.

Address comments.
Only complete last segment for better compatibility.


Repository:
  rC Clang

https://reviews.llvm.org/D52076

Files:
  include/clang-c/Index.h
  include/clang/Lex/CodeCompletionHandler.h
  include/clang/Lex/Lexer.h
  include/clang/Lex/Preprocessor.h
  include/clang/Parse/Parser.h
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Lex/Lexer.cpp
  lib/Lex/Preprocessor.cpp
  lib/Parse/Parser.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/included-files.cpp
  tools/libclang/CIndexCodeCompletion.cpp

Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -499,6 +499,10 @@
   contexts = CXCompletionContext_NaturalLanguage;
   break;
 }
+case CodeCompletionContext::CCC_IncludedFile: {
+  contexts = CXCompletionContext_IncludedFile;
+  break;
+}
 case CodeCompletionContext::CCC_SelectorName: {
   contexts = CXCompletionContext_ObjCSelectorName;
   break;
Index: test/CodeCompletion/included-files.cpp
===
--- /dev/null
+++ test/CodeCompletion/included-files.cpp
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/a
+// RUN: touch %t/foo.h && touch %t/foo.cc && touch %t/a/foosys %t/a/foosys.h
+
+// Quoted string shows header-ish files from CWD, and all from system.
+#include "foo.h"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:5:13 %t/main.cc | FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1-NOT: foo.cc"
+// CHECK-1: foo.h"
+// CHECK-1: foosys"
+
+// Quoted string with dir shows header-ish files in that subdir.
+#include "a/foosys"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:12:13 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
+// CHECK-2-NOT: foo.h"
+// CHECK-2: foosys.h"
+// CHECK-2-NOT: foosys"
+
+// Angled string showes all files, but only in system dirs.
+#include 
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:19:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
+// CHECK-3-NOT: foo.cc>
+// CHECK-3-NOT: foo.h>
+// CHECK-3: foosys>
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -32,6 +32,8 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Path.h"
 #include 
 #include 
 #include 
@@ -7994,6 +7996,114 @@
   // for the expanded tokens.
 }
 
+// This handles completion inside an #include filename, e.g. #include  NativeRelDir = StringRef(RelDir);
+  llvm::sys::path::native(NativeRelDir);
+  auto FS = getSourceManager().getFileManager().getVirtualFileSystem();
+
+  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo(),
+CodeCompletionContext::CCC_IncludedFile);
+  llvm::DenseSet SeenResults; // To deduplicate results.
+
+  // Helper: adds one file or directory completion result.
+  auto AddCompletion = [&](StringRef Filename, bool IsDirectory) {
+SmallString<64> TypedChunk = Filename;
+// Directory completion is up to the slash, e.g. ' : '"');
+auto R = SeenResults.insert(TypedChunk);
+if (R.second) { // New completion
+  const char *InternedTyped = Results.getAllocator().CopyString(TypedChunk);
+  *R.first = InternedTyped; // Avoid dangling StringRef.
+  CodeCompletionBuilder Builder(CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo());
+  Builder.AddTypedTextChunk(InternedTyped);
+  // The result is a "Pattern", which is pretty opaque.
+  // We may want to include the real filename to allow smart ranking.
+  Results.AddResult(CodeCompletionResult(Builder.TakeString()));
+}
+  };
+
+  // Helper: scans IncludeDir for nice files, and adds results for each.
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem) {
+llvm::SmallString<128> Dir = IncludeDir;
+if (!NativeRelDir.empty())
+  llvm::sys::path::append(Dir, NativeRelDir);
+
+std::error_code EC;
+unsigned Count = 0;
+for (auto It = FS->dir_begin(Dir, EC);
+ !EC && It != vfs::directory_iterator(); It.increment(EC)) {
+  if (++Count == 2500) // If we happen to hit a huge directory,
+break; // bail out early so we're not too slow.
+  StringRef Filename = llvm::sys::path::filename(It->path());
+  switch (It->type()) {
+  case 

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

2018-09-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Sorry for the late response.

Please see the comments about adding closing quotes and slashes. It does not 
seem to work in the clients that auto-add closing quotes or when completing 
inside existing includes.




Comment at: lib/Lex/Lexer.cpp:1931
+  StringRef PartialPath(AfterQuote, CurPtr - 1 - AfterQuote);
+  auto Slash = PartialPath.rfind('/');
+  StringRef Dir =

sammccall wrote:
> ilya-biryukov wrote:
> > It's also valid to use `\` for path separators on Windows, maybe also 
> > handle those?
> > Clang also supports this when running e.g. `clang-cl /c foo.cpp`.
> Ah yuck, that's true (for some value of "valid"). Pre-C++11 it was explicitly 
> UB...
> 
> So if a codebase consistently uses backslashes, we're going to break that 
> style at least sometimes: we can't know whether to complete `#include  as ` 
> Any objection to just normalizing slashes for the whole include as part of 
> completion?
> e.g. `#include  `?
> 
> Anything else is going to be fiddly, probably only help a few users a little 
> bit, and leave edge cases.
If we normalize, we probably want a config option for Windows users to be 
friendly to them.
No good layer to add the option, though (except a compiler flag, but that looks 
weird), so I don't think that's directly addressable. 

Also, do we want to actually change slashes **before** the path component we 
are completing in the same include directive? 
1. If we do, I suspect we might have problems with the existing language 
clients. We ran into problems when trying to add `.` to `->` fix-its.
2. If we don't, the completion label would look differently from what's 
written. That's probably not a big deal.



Comment at: lib/Lex/Lexer.cpp:2065
+if (C == '\n' || C == '\r' ||// Newline.
+(C == 0 && (CurPtr - 1 == BufferEnd))) { // End of file.
   // If the filename is unterminated, then it must just be a lone <

sammccall wrote:
> ilya-biryukov wrote:
> > Does that mean we're losing completion at the end of file?
> > Not sure if it's a big deal. The only common pattern I can come up with is 
> > starting with an empty file and writing:
> > ```
> > #include "^
> > ``` 
> > 
> > Should we handle that? WDYT?
> Ah no, it doesn't, though I thought so at first.
> 
> First: the address of `C` is `CurPtr-1` (getAndAdvanceChar is in spirit 
> `return *CurPtr++`).
> 
> Second: the buffer is null-terminated one-past-the-end (`*BufferEnd == 0`) 
> and **also** has an embedded null at the completion point. So when completing 
> at EOF it looks like:
> ```
> # i n c l u d e < a \0 \0
> ^C ^CurPtr
> ^BufferStart^BufPtr^BufferEnd
> ```
> and we hit the case below. We only hit this case if we never saw a completion 
> point.
Ah, I see. Thanks for explanation. LG



Comment at: lib/Sema/SemaCodeComplete.cpp:8021
+// Directory completion is up to the slash, e.g. ' : '"');
+auto R = SeenResults.insert(TypedChunk);

sammccall wrote:
> ilya-biryukov wrote:
> > Could we avoid adding `/`, `>` and `"` if it's currently missing?
> > Otherwise it'll be annoying for editors that auto-add closing quotes and 
> > completions when editing existing includes, i.e.
> > ```
> > // It's cool if we complete bar.h" (with closing quote) here:
> > #include "foo/^
> > // It's annoying if we complete an extra closing quote here:
> > #include "foo/^"
> > ```
> Don't editors that add quotes/parens typically avoid duplicating them when 
> they're typed/completed?
> 
> This isn't actually easy to do, because after completing `file.h` you want 
> the cursor after the closing quote, and after completing `dir/` you want the 
> cursor inside the closing quote. I don't see a way to do this with 
> CodeCompletionString other than generating the quote in the former case but 
> not the latter. (At least not one that doesn't break something else...)
> Don't editors that add quotes/parens typically avoid duplicating them when 
> they're typed/completed?
They do that on typing, but not on completions, which are treated literally, 
e.g. they'll an extra quote is actually added.
I've checked in VSCode and it produces double quotes in those cases. Not sure 
about the other editors that have this.
That looks like a corner case my intuition is that other editors won't do what 
we want either.



Comment at: lib/Sema/SemaCodeComplete.cpp:8028
+CodeCompleter->getCodeCompletionTUInfo());
+  Builder.AddTextChunk(InternedPrefix);
+  Builder.AddTypedTextChunk(InternedTyped);

sammccall wrote:
> ilya-biryukov wrote:
> > Maybe leave this out?
> > When completing `std::^` the completion is `vector`, not `std::vector`.
> > In the same spirit, for includes I would expect completions for `#include 
> > ` to be `bar.h`, not ``.
> I tried a few iterations here and 

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

2018-09-14 Thread 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-09-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 165593.
sammccall marked 9 inline comments as done.
sammccall added a comment.

Unify common completion code from angled/quoted strings in Lexer.
Handle #include paths with \ on windows (normalize them to /)
Document why we picked particular extensions for headers.
Increase per-dir listing limit to 2500.

1. Updating https://reviews.llvm.org/D52076: [CodeComplete] Add completions for 
filenames in #include directives. #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

:


Repository:
  rC Clang

https://reviews.llvm.org/D52076

Files:
  include/clang-c/Index.h
  include/clang/Lex/CodeCompletionHandler.h
  include/clang/Lex/Lexer.h
  include/clang/Lex/Preprocessor.h
  include/clang/Parse/Parser.h
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Lex/Lexer.cpp
  lib/Lex/Preprocessor.cpp
  lib/Parse/Parser.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/included-files.cpp
  tools/libclang/CIndexCodeCompletion.cpp

Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -499,6 +499,10 @@
   contexts = CXCompletionContext_NaturalLanguage;
   break;
 }
+case CodeCompletionContext::CCC_IncludedFile: {
+  contexts = CXCompletionContext_IncludedFile;
+  break;
+}
 case CodeCompletionContext::CCC_SelectorName: {
   contexts = CXCompletionContext_ObjCSelectorName;
   break;
Index: test/CodeCompletion/included-files.cpp
===
--- /dev/null
+++ test/CodeCompletion/included-files.cpp
@@ -0,0 +1,25 @@
+// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/a
+// RUN: touch %t/foo.h && touch %t/foo.cc && touch %t/a/foosys %t/a/foosys.h
+
+// Quoted string shows header-ish files from CWD, and all from system.
+#include "foo.h"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:5:13 %t/main.cc | FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1-NOT: "foo.cc"
+// CHECK-1: "foo.h"
+// CHECK-1: "foosys"
+
+// Quoted string with dir shows header-ish files in that subdir.
+#include "a/foosys"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:12:13 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
+// CHECK-2-NOT: "a/foosys"
+// CHECK-2: "a/foosys.h"
+// CHECK-2-NOT: "a/foosys"
+// CHECK-2-NOT: "foo.h"
+// CHECK-2-NOT: "foosys"
+
+// Angled string showes all files, but only in system dirs.
+#include 
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:21:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
+// CHECK-3-NOT: 
+// CHECK-3-NOT: 
+// CHECK-3: 
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -32,6 +32,8 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Path.h"
 #include 
 #include 
 #include 
@@ -7994,6 +7996,120 @@
   // for the expanded tokens.
 }
 
+// This handles completion inside an #include filename, e.g. #include getAllocator().CopyString(
+ (Angled ? "<" : "\"") + RelDir + "/");
+  // We need the native slashes for the actual file system interactions.
+  SmallString<128> NativeRelDir = StringRef(RelDir);
+  llvm::sys::path::native(NativeRelDir);
+  auto FS = getSourceManager().getFileManager().getVirtualFileSystem();
+
+  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo(),
+CodeCompletionContext::CCC_IncludedFile);
+  llvm::DenseSet SeenResults; // To deduplicate results.
+
+  // Helper: adds one file or directory completion result.
+  auto AddCompletion = [&](StringRef Filename, bool IsDirectory) {
+SmallString<64> TypedChunk = Filename;
+// Directory completion is up to the slash, e.g. ' : '"');
+auto R = SeenResults.insert(TypedChunk);
+if (R.second) { // New completion
+  const char *InternedTyped = Results.getAllocator().CopyString(TypedChunk);
+  *R.first = InternedTyped; // Avoid dangling StringRef.
+  CodeCompletionBuilder Builder(CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo());
+  Builder.AddTextChunk(InternedPrefix);
+  Builder.AddTypedTextChunk(InternedTyped);
+  // The result is a "Pattern", which is pretty opaque.
+  // We may want to include the real filename to allow smart ranking.

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

2018-09-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Thanks a lot, great comments!
I haven't made changes yet (I will) but there's a few questions to work out 
first...




Comment at: lib/Lex/Lexer.cpp:1931
+  StringRef PartialPath(AfterQuote, CurPtr - 1 - AfterQuote);
+  auto Slash = PartialPath.rfind('/');
+  StringRef Dir =

ilya-biryukov wrote:
> It's also valid to use `\` for path separators on Windows, maybe also handle 
> those?
> Clang also supports this when running e.g. `clang-cl /c foo.cpp`.
Ah yuck, that's true (for some value of "valid"). Pre-C++11 it was explicitly 
UB...

So if a codebase consistently uses backslashes, we're going to break that style 
at least sometimes: we can't know whether to complete `#include `?

Anything else is going to be fiddly, probably only help a few users a little 
bit, and leave edge cases.



Comment at: lib/Lex/Lexer.cpp:2065
+if (C == '\n' || C == '\r' ||// Newline.
+(C == 0 && (CurPtr - 1 == BufferEnd))) { // End of file.
   // If the filename is unterminated, then it must just be a lone <

ilya-biryukov wrote:
> Does that mean we're losing completion at the end of file?
> Not sure if it's a big deal. The only common pattern I can come up with is 
> starting with an empty file and writing:
> ```
> #include "^
> ``` 
> 
> Should we handle that? WDYT?
Ah no, it doesn't, though I thought so at first.

First: the address of `C` is `CurPtr-1` (getAndAdvanceChar is in spirit `return 
*CurPtr++`).

Second: the buffer is null-terminated one-past-the-end (`*BufferEnd == 0`) and 
**also** has an embedded null at the completion point. So when completing at 
EOF it looks like:
```
# i n c l u d e < a \0 \0
^C ^CurPtr
^BufferStart^BufPtr^BufferEnd
```
and we hit the case below. We only hit this case if we never saw a completion 
point.



Comment at: lib/Sema/SemaCodeComplete.cpp:8021
+// Directory completion is up to the slash, e.g. ' : '"');
+auto R = SeenResults.insert(TypedChunk);

ilya-biryukov wrote:
> Could we avoid adding `/`, `>` and `"` if it's currently missing?
> Otherwise it'll be annoying for editors that auto-add closing quotes and 
> completions when editing existing includes, i.e.
> ```
> // It's cool if we complete bar.h" (with closing quote) here:
> #include "foo/^
> // It's annoying if we complete an extra closing quote here:
> #include "foo/^"
> ```
Don't editors that add quotes/parens typically avoid duplicating them when 
they're typed/completed?

This isn't actually easy to do, because after completing `file.h` you want the 
cursor after the closing quote, and after completing `dir/` you want the cursor 
inside the closing quote. I don't see a way to do this with 
CodeCompletionString other than generating the quote in the former case but not 
the latter. (At least not one that doesn't break something else...)



Comment at: lib/Sema/SemaCodeComplete.cpp:8028
+CodeCompleter->getCodeCompletionTUInfo());
+  Builder.AddTextChunk(InternedPrefix);
+  Builder.AddTypedTextChunk(InternedTyped);

ilya-biryukov wrote:
> Maybe leave this out?
> When completing `std::^` the completion is `vector`, not `std::vector`.
> In the same spirit, for includes I would expect completions for `#include 
> ` to be `bar.h`, not ``.
I tried a few iterations here and this was the only one that didn't seem 
totally odd. Basically it's about the fact that these are quoted strings.

Ergonomically, having the closing quote completed after a filename *feels* 
right - you can hit enter afterwards and go to the next line.
If the closing quote is inserted, it has to be displayed (can't be avoided, 
also would be confusing).

So if you make the completion range just the filename, you end up with a 
completion list like:
```
#include 
  baz/
  backwards.h>  
```
vs the current
```
#include 
   
```

So I see your point here but I'm not sure the other behavior is actually better.



Comment at: lib/Sema/SemaCodeComplete.cpp:8057
+  if (!(Filename.endswith(".h") || Filename.endswith(".hh") ||
+Filename.endswith(".H") || Filename.endswith(".hpp") ||
+Filename.endswith(".inc")))

ilya-biryukov wrote:
> Maybe do case-insensitive matching?
> A corner case, but still...
This matches Driver's behavior (I'd reuse it but I don't think Sema can/should 
depend on Driver). Clang doesn't think "HPP" is a valid c++ header filename.
I'll add a comment.


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


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

2018-09-14 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Amazing, can't wait for it to land!




Comment at: lib/Lex/Lexer.cpp:1931
+  StringRef PartialPath(AfterQuote, CurPtr - 1 - AfterQuote);
+  auto Slash = PartialPath.rfind('/');
+  StringRef Dir =

It's also valid to use `\` for path separators on Windows, maybe also handle 
those?
Clang also supports this when running e.g. `clang-cl /c foo.cpp`.



Comment at: lib/Lex/Lexer.cpp:2065
+if (C == '\n' || C == '\r' ||// Newline.
+(C == 0 && (CurPtr - 1 == BufferEnd))) { // End of file.
   // If the filename is unterminated, then it must just be a lone <

Does that mean we're losing completion at the end of file?
Not sure if it's a big deal. The only common pattern I can come up with is 
starting with an empty file and writing:
```
#include "^
``` 

Should we handle that? WDYT?



Comment at: lib/Lex/Lexer.cpp:2075
+// Completion only applies to the filename, after the last slash.
+StringRef PartialPath(AfterLessPos, CurPtr - 1 - AfterLessPos);
+auto Slash = PartialPath.rfind('/');

This code looks exactly the same between two functions.
Maybe extract it into a helper function?



Comment at: lib/Sema/SemaCodeComplete.cpp:8021
+// Directory completion is up to the slash, e.g. ' : '"');
+auto R = SeenResults.insert(TypedChunk);

Could we avoid adding `/`, `>` and `"` if it's currently missing?
Otherwise it'll be annoying for editors that auto-add closing quotes and 
completions when editing existing includes, i.e.
```
// It's cool if we complete bar.h" (with closing quote) here:
#include "foo/^
// It's annoying if we complete an extra closing quote here:
#include "foo/^"
```



Comment at: lib/Sema/SemaCodeComplete.cpp:8028
+CodeCompleter->getCodeCompletionTUInfo());
+  Builder.AddTextChunk(InternedPrefix);
+  Builder.AddTypedTextChunk(InternedTyped);

Maybe leave this out?
When completing `std::^` the completion is `vector`, not `std::vector`.
In the same spirit, for includes I would expect completions for `#include 
` to be `bar.h`, not ``.



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.

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?



Comment at: lib/Sema/SemaCodeComplete.cpp:8057
+  if (!(Filename.endswith(".h") || Filename.endswith(".hh") ||
+Filename.endswith(".H") || Filename.endswith(".hpp") ||
+Filename.endswith(".inc")))

Maybe do case-insensitive matching?
A corner case, but still...



Comment at: lib/Sema/SemaCodeComplete.cpp:8076
+  // header maps are not (currently) enumerable.
+  break;
+case DirectoryLookup::LT_NormalDir:

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.


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


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

2018-09-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added a subscriber: cfe-commits.

The dir component ("somedir" in #include ) is considered fixed.
We append "foo" to each directory on the include path, and then list its files.

Completions are of the forms:
 
 ^-text--^^-TT-^  (TypedText)
and
 https://reviews.llvm.org/D51921 will fix.


Repository:
  rC Clang

https://reviews.llvm.org/D52076

Files:
  include/clang-c/Index.h
  include/clang/Lex/CodeCompletionHandler.h
  include/clang/Lex/Preprocessor.h
  include/clang/Parse/Parser.h
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Lex/Lexer.cpp
  lib/Lex/Preprocessor.cpp
  lib/Parse/Parser.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/included-files.cpp
  tools/libclang/CIndexCodeCompletion.cpp

Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -499,6 +499,10 @@
   contexts = CXCompletionContext_NaturalLanguage;
   break;
 }
+case CodeCompletionContext::CCC_IncludedFile: {
+  contexts = CXCompletionContext_IncludedFile;
+  break;
+}
 case CodeCompletionContext::CCC_SelectorName: {
   contexts = CXCompletionContext_ObjCSelectorName;
   break;
Index: test/CodeCompletion/included-files.cpp
===
--- /dev/null
+++ test/CodeCompletion/included-files.cpp
@@ -0,0 +1,25 @@
+// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/a
+// RUN: touch %t/foo.h && touch %t/foo.cc && touch %t/a/foosys %t/a/foosys.h
+
+// Quoted string shows header-ish files from CWD, and all from system.
+#include "foo.h"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:5:13 %t/main.cc | FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1-NOT: "foo.cc"
+// CHECK-1: "foo.h"
+// CHECK-1: "foosys"
+
+// Quoted string with dir shows header-ish files in that subdir.
+#include "a/foosys"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:12:13 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
+// CHECK-2-NOT: "a/foosys"
+// CHECK-2: "a/foosys.h"
+// CHECK-2-NOT: "a/foosys"
+// CHECK-2-NOT: "foo.h"
+// CHECK-2-NOT: "foosys"
+
+// Angled string showes all files, but only in system dirs.
+#include 
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:21:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
+// CHECK-3-NOT: 
+// CHECK-3-NOT: 
+// CHECK-3: 
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -32,6 +32,8 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Path.h"
 #include 
 #include 
 #include 
@@ -7994,6 +7996,115 @@
   // for the expanded tokens.
 }
 
+// This handles completion inside an #include filename, e.g. #include getAllocator().CopyString(
+ (Angled ? "<" : "\"") + RelDir + "/");
+  SmallString<128> NativeRelDir = RelDir;
+  llvm::sys::path::native(NativeRelDir);
+  auto FS = getSourceManager().getFileManager().getVirtualFileSystem();
+
+  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo(),
+CodeCompletionContext::CCC_IncludedFile);
+  llvm::DenseSet SeenResults; // To deduplicate results.
+
+  // Helper: adds one file or directory completion result.
+  auto AddCompletion = [&](StringRef Filename, bool IsDirectory) {
+SmallString<64> TypedChunk = Filename;
+// Directory completion is up to the slash, e.g. ' : '"');
+auto R = SeenResults.insert(TypedChunk);
+if (R.second) { // New completion
+  const char *InternedTyped = Results.getAllocator().CopyString(TypedChunk);
+  *R.first = InternedTyped; // Avoid dangling StringRef.
+  CodeCompletionBuilder Builder(CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo());
+  Builder.AddTextChunk(InternedPrefix);
+  Builder.AddTypedTextChunk(InternedTyped);
+  // The result is a "Pattern", which is pretty opaque.
+  // We may want to include the real filename to allow smart ranking.
+  Results.AddResult(CodeCompletionResult(Builder.TakeString()));
+}
+  };
+
+  // Helper: scans IncludeDir for nice files, and adds results for each.
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem) {
+llvm::SmallString<128> Dir = IncludeDir;
+if (!NativeRelDir.empty())
+  llvm::sys::path::append(Dir, NativeRelDir);
+
+std::error_code EC;
+unsigned Count =