[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

I have a similar structure to Qt regarding the headers and use clangd trough 
the QtCreator, so for my own libraries I cannot use the auto completion.

In D112996#3102782 , @sammccall wrote:

>> the files are located in include directories
>
> These are directories that **may** contain headers, not directories that 
> **only** contain headers. (Which we mostly expect `-Isystem` to be).
> For example, many projects keep headers next to sources, and so have sources 
> on their include path. And the directory containing the current file is 
> always on the include path.
> We'd expect to see `Makefile`, `SConstruct`, `BUILD` files etc here. And 
> maybe a smattering of "random" files that don't follow particular conventions.
>
> Trying to support QT headers seems very reasonable though. Seems like our 
> options are:
>
> - current behavior with false negatives on QT
> - proposed behavior with false positives on Makefile etc
> - current behavior and try to detect QT as an exception
> - proposed behavior and try to detect Makefile etc as exceptions
>
> WDYT about detecting QT headers specifically? It seems hacky, but I don't see 
> a way out of this that doesn't involve hardcoding some filenames. Are they in 
> a directory like `"qt-11/QFoo"` that we can recognize? Even Q followed by 
> another capital letter might be a good enough heuristic.
> (The docs suggest it's just `` but the docs also say to use angle 
> brackets so I'm not sure whether to believe them)

Could we extend the heuristic to match directories called `include`? For me it 
is

  Path to Library   .../code/cast
  - include .../code/cast/include
- LibraryName   .../code/cast/include/Cast
  - Header  .../code/cast/include/Cast/Signed

And for that I want `#include `.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks Christian! Sorry for the delay.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf7500a4ef7bd: [CodeCompletion] Generally consider header 
files without extension (authored by ckandeler, committed by sammccall).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/included-files.cpp

Index: clang/test/CodeCompletion/included-files.cpp
===
--- clang/test/CodeCompletion/included-files.cpp
+++ clang/test/CodeCompletion/included-files.cpp
@@ -1,35 +1,57 @@
-// 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
+// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/a && mkdir %t/QtCore && mkdir %t/Headers %t/Some.framework %t/Some.framework/Headers
+// RUN: touch %t/foo.h && touch %t/foo.cc && touch %t/a/foosys %t/a/foosys.h && touch %t/QtCore/foosys %t/QtCore/foo.h
+// RUN: touch %t/Headers/foosys %t/Headers/foo.h %t/Some.framework/Headers/foosys %t/Some.framework/Headers/foo.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
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:6: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
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:13:13 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
 // CHECK-2-NOT: foo.h"
 // CHECK-2: foosys.h"
 // CHECK-2-NOT: foosys"
 
 // Angled shows headers from 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
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:20:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
 // CHECK-3-NOT: foo.cc>
 // CHECK-3-NOT: foo.h>
 // CHECK-3: foosys>
 
 // With -I rather than -isystem, the header extension is required.
 #include 
-// RUN: %clang -fsyntax-only -I %t/a -Xclang -code-completion-at=%t/main.cc:26:13 %t/main.cc | FileCheck -check-prefix=CHECK-4 %s
+// RUN: %clang -fsyntax-only -I %t/a -Xclang -code-completion-at=%t/main.cc:27:13 %t/main.cc | FileCheck -check-prefix=CHECK-4 %s
 // CHECK-4-NOT: foo.cc>
 // CHECK-4-NOT: foo.h>
 // CHECK-4-NOT: foosys>
 
 // Backslash handling.
 #include "a\foosys"
-// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:33:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-5 %s
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:34:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-5 %s
 // CHECK-5: foosys.h"
+
+// Qt headers don't necessarily have extensions.
+#include 
+// RUN: %clang -fsyntax-only -I %t/QtCore -Xclang -code-completion-at=%t/main.cc:39:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-6 %s
+// CHECK-6-NOT: foo.cc>
+// CHECK-6: foo.h>
+// CHECK-6: foosys>
+
+// If the include path directly points into a framework's Headers/ directory, we allow extension-less headers.
+#include 
+// RUN: %clang -fsyntax-only -I %t/Some.framework/Headers -Xclang -code-completion-at=%t/main.cc:46:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-7 %s
+// CHECK-7-NOT: foo.cc>
+// CHECK-7: foo.h>
+// CHECK-7: foosys>
+
+// Simply naming a directory "Headers" is not enough to allow extension-less headers.
+#include 
+// RUN: %clang -fsyntax-only -I %t/Headers -Xclang -code-completion-at=%t/main.cc:53:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-8 %s
+// CHECK-8-NOT: foo.cc>
+// CHECK-8: foo.h>
+// CHECK-8-NOT: foosys>
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef  = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders =
+IsSystem || isQt || Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,18 +9643,19 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that 

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 385826.
ckandeler added a comment.

Added test cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/included-files.cpp

Index: clang/test/CodeCompletion/included-files.cpp
===
--- clang/test/CodeCompletion/included-files.cpp
+++ clang/test/CodeCompletion/included-files.cpp
@@ -1,35 +1,57 @@
-// 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
+// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/a && mkdir %t/QtCore && mkdir %t/Headers %t/Some.framework %t/Some.framework/Headers
+// RUN: touch %t/foo.h && touch %t/foo.cc && touch %t/a/foosys %t/a/foosys.h && touch %t/QtCore/foosys %t/QtCore/foo.h
+// RUN: touch %t/Headers/foosys %t/Headers/foo.h %t/Some.framework/Headers/foosys %t/Some.framework/Headers/foo.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
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:6: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
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:13:13 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
 // CHECK-2-NOT: foo.h"
 // CHECK-2: foosys.h"
 // CHECK-2-NOT: foosys"
 
 // Angled shows headers from 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
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:20:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
 // CHECK-3-NOT: foo.cc>
 // CHECK-3-NOT: foo.h>
 // CHECK-3: foosys>
 
 // With -I rather than -isystem, the header extension is required.
 #include 
-// RUN: %clang -fsyntax-only -I %t/a -Xclang -code-completion-at=%t/main.cc:26:13 %t/main.cc | FileCheck -check-prefix=CHECK-4 %s
+// RUN: %clang -fsyntax-only -I %t/a -Xclang -code-completion-at=%t/main.cc:27:13 %t/main.cc | FileCheck -check-prefix=CHECK-4 %s
 // CHECK-4-NOT: foo.cc>
 // CHECK-4-NOT: foo.h>
 // CHECK-4-NOT: foosys>
 
 // Backslash handling.
 #include "a\foosys"
-// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:33:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-5 %s
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:34:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-5 %s
 // CHECK-5: foosys.h"
+
+// Qt headers don't necessarily have extensions.
+#include 
+// RUN: %clang -fsyntax-only -I %t/QtCore -Xclang -code-completion-at=%t/main.cc:39:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-6 %s
+// CHECK-6-NOT: foo.cc>
+// CHECK-6: foo.h>
+// CHECK-6: foosys>
+
+// If the include path directly points into a framework's Headers/ directory, we allow extension-less headers.
+#include 
+// RUN: %clang -fsyntax-only -I %t/Some.framework/Headers -Xclang -code-completion-at=%t/main.cc:46:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-7 %s
+// CHECK-7-NOT: foo.cc>
+// CHECK-7: foo.h>
+// CHECK-7: foosys>
+
+// Simply naming a directory "Headers" is not enough to allow extension-less headers.
+#include 
+// RUN: %clang -fsyntax-only -I %t/Headers -Xclang -code-completion-at=%t/main.cc:53:13 %t/main.cc -fms-compatibility | FileCheck -check-prefix=CHECK-8 %s
+// CHECK-8-NOT: foo.cc>
+// CHECK-8: foo.h>
+// CHECK-8-NOT: foosys>
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef  = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders =
+IsSystem || isQt || Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,18 +9643,19 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, which we 

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D112996#3106469 , @ckandeler wrote:

> In D112996#3106459 , @sammccall 
> wrote:
>
>> can we the tests again though?
>
> Sorry, I don't understand what you mean by that.

Apologies, I mangled the comment.

This should have a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Can someone please merge this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 385453.
ckandeler added a comment.

Addressed formatting comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef  = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders =
+IsSystem || isQt || Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,18 +9643,19 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, which we can't depend on here.
-  if (!(Filename.endswith_insensitive(".h") ||
-Filename.endswith_insensitive(".hh") ||
-Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
-break;
-}
+  case llvm::sys::fs::file_type::regular_file: {
+// Only files that really look like headers. (Except in special dirs).
+// Header extensions from Types.def, which we can't depend on here.
+const bool IsHeader = Filename.endswith_insensitive(".h") ||
+  Filename.endswith_insensitive(".hh") ||
+  Filename.endswith_insensitive(".hpp") ||
+  Filename.endswith_insensitive(".inc") ||
+  (ExtensionlessHeaders && 
!Filename.contains('.'));
+if (!IsHeader)
+  break;
 AddCompletion(Filename, /*IsDirectory=*/false);
 break;
+  }
   default:
 break;
   }


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef  = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders =
+IsSystem || isQt || Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,18 +9643,19 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, which we can't depend on here.
-  if (!(Filename.endswith_insensitive(".h") ||
-Filename.endswith_insensitive(".hh") ||
-Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
-break;
-}
+  case llvm::sys::fs::file_type::regular_file: {
+// Only files that really look like headers. (Except in special dirs).
+// Header extensions from Types.def, which we can't depend on here.
+const bool IsHeader = Filename.endswith_insensitive(".h") ||
+  Filename.endswith_insensitive(".hh") ||
+  Filename.endswith_insensitive(".hpp") ||
+  Filename.endswith_insensitive(".inc") ||
+  (ExtensionlessHeaders && !Filename.contains('.'));
+if (!IsHeader)
+  break;
 AddCompletion(Filename, /*IsDirectory=*/false);
 break;
+  }
   default:
 break;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D112996#3106459 , @sammccall wrote:

> can we the tests again though?

Sorry, I don't understand what you mean by that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:9619
+const bool ExtensionlessHeaders = IsSystem || isQt
+|| Dir.endswith(".framework/Headers");
 std::error_code EC;

I'm just noticing that this is often a symlink into a versioned directory (e.g. 
xxx.framework/Version/5/Headers). The include paths are not canonicalized, I 
hope?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

LG, thanks! can we the tests again though?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 384485.
ckandeler added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef  = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders = IsSystem || isQt
+|| Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,15 +9643,16 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, which we can't depend on here.
-  if (!(Filename.endswith_insensitive(".h") ||
-Filename.endswith_insensitive(".hh") ||
-Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
-break;
+  case llvm::sys::fs::file_type::regular_file: {
+// Only files that really look like headers. (Except in special dirs).
+// Header extensions from Types.def, which we can't depend on here.
+const bool IsHeader = Filename.endswith_insensitive(".h") ||
+Filename.endswith_insensitive(".hh") ||
+Filename.endswith_insensitive(".hpp") ||
+Filename.endswith_insensitive(".inc") ||
+(ExtensionlessHeaders && !Filename.contains('.'));
+if (!IsHeader)
+  break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);
 break;


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9613,6 +9613,10 @@
   }
 }
 
+const StringRef  = llvm::sys::path::filename(Dir);
+const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
+const bool ExtensionlessHeaders = IsSystem || isQt
+|| Dir.endswith(".framework/Headers");
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9639,15 +9643,16 @@
 
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
-  case llvm::sys::fs::file_type::regular_file:
-// Only files that really look like headers. (Except in system dirs).
-if (!IsSystem) {
-  // Header extensions from Types.def, which we can't depend on here.
-  if (!(Filename.endswith_insensitive(".h") ||
-Filename.endswith_insensitive(".hh") ||
-Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
-break;
+  case llvm::sys::fs::file_type::regular_file: {
+// Only files that really look like headers. (Except in special dirs).
+// Header extensions from Types.def, which we can't depend on here.
+const bool IsHeader = Filename.endswith_insensitive(".h") ||
+Filename.endswith_insensitive(".hh") ||
+Filename.endswith_insensitive(".hpp") ||
+Filename.endswith_insensitive(".inc") ||
+(ExtensionlessHeaders && !Filename.contains('.'));
+if (!IsHeader)
+  break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D112996#3105783 , @ckandeler wrote:

>>> For framework builds, the directory would be "Headers", which also seems 
>>> safe.
>>
>> I agree extensionless headers in frameworks seem fine to show.
>> We already know which includepath entries are frameworks, so we can just use 
>> that info directly (see line 9674)
>
> These aren't technically framework includes, as in order to make prefix-less 
> headers work, the include path has to point directly into the Headers 
> directory.

I see. In that case we can detect this pattern in any framework by checking 
whether the directory contains ".framework/Headers", right? Headers itself 
doesn't seem specific enough.




Comment at: clang/lib/Sema/SemaCodeComplete.cpp:9654
+Dirname == "Headers" ||
+Dirname.startswith("Qt") || Dirname == "ActiveQt"))
 break;

can you extract this up near dirname, and give a name that explains the idea 
(e.g. `ExtensionlessHeaders`)



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:9654
+Dirname == "Headers" ||
+Dirname.startswith("Qt") || Dirname == "ActiveQt"))
 break;

sammccall wrote:
> can you extract this up near dirname, and give a name that explains the idea 
> (e.g. `ExtensionlessHeaders`)
we've lost the condition that there's no extension, and now accept *any* 
extension (e.g. `.DS_Store`!)
Unless this is important I think we should keep that check.

And to keep the number of cases/paths low, I think it's fine to bundle system 
headers into the same bucket - really this is just for the C++ standard 
library, which does not have extensions.

So some logic like:

```
bool ExtensionlessHeaders = IsSystem || Dir.endswith(".framework/Headers") || 
Dirname.startswith("Qt");
...
case regular_file:
  bool IsHeader = Filename.endswith(...) || Filename.endswith(...) || 
(ExtensionlessHeaders && !Filename.contains('.'));
  if (!IsHeader) break;
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

>> For framework builds, the directory would be "Headers", which also seems 
>> safe.
>
> I agree extensionless headers in frameworks seem fine to show.
> We already know which includepath entries are frameworks, so we can just use 
> that info directly (see line 9674)

These aren't technically framework includes, as in order to make prefix-less 
headers work, the include path has to point directly into the Headers directory.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 384407.
ckandeler added a comment.

Limited the matching to Qt headers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9615,6 +9615,7 @@
   }
 }
 
+const StringRef  = llvm::sys::path::filename(Dir);
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9648,7 +9649,9 @@
   if (!(Filename.endswith_insensitive(".h") ||
 Filename.endswith_insensitive(".hh") ||
 Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
+Filename.endswith_insensitive(".inc") ||
+Dirname == "Headers" ||
+Dirname.startswith("Qt") || Dirname == "ActiveQt"))
 break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9615,6 +9615,7 @@
   }
 }
 
+const StringRef  = llvm::sys::path::filename(Dir);
 std::error_code EC;
 unsigned Count = 0;
 for (auto It = FS.dir_begin(Dir, EC);
@@ -9648,7 +9649,9 @@
   if (!(Filename.endswith_insensitive(".h") ||
 Filename.endswith_insensitive(".hh") ||
 Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
+Filename.endswith_insensitive(".inc") ||
+Dirname == "Headers" ||
+Dirname.startswith("Qt") || Dirname == "ActiveQt"))
 break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D112996#3102925 , @ckandeler wrote:

>> WDYT about detecting QT headers specifically? It seems hacky, but I don't 
>> see a way out of this that doesn't involve hardcoding some filenames. Are 
>> they in a directory like `"qt-11/QFoo"` that we can recognize? Even Q 
>> followed by another capital letter might be a good enough heuristic.
>> (The docs suggest it's just `` but the docs also say to use angle 
>> brackets so I'm not sure whether to believe them)
>
> The headers are, as far as I can tell, always located in a directory whose 
> name starts with "Qt". This parent directory is also in the include path, so 
> e.g. to get access to QString, which is located under QtCore/, you'd 
> typically just write:
>
>   #include 
>
> This is the recommended, documented way of pulling in headers.
> Though you could also write:
>
>   #include 
>
> as the parent parent directory is also in the list of include paths.
>
> Looking at the code, it seems we have access to the parent directory, so we 
> could do that name check (which I suppose has less potential for false 
> positives than checking the file name).

Yeah, that makes sense to me. Also a bit cheaper since we only have to do this 
once per parent dir.

> For framework builds, the directory would be "Headers", which also seems safe.

I agree extensionless headers in frameworks seem fine to show.
We already know which includepath entries are frameworks, so we can just use 
that info directly (see line 9674)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

> WDYT about detecting QT headers specifically? It seems hacky, but I don't see 
> a way out of this that doesn't involve hardcoding some filenames. Are they in 
> a directory like `"qt-11/QFoo"` that we can recognize? Even Q followed by 
> another capital letter might be a good enough heuristic.
> (The docs suggest it's just `` but the docs also say to use angle 
> brackets so I'm not sure whether to believe them)

The headers are, as far as I can tell, always located in a directory whose name 
starts with "Qt". This parent directory is also in the include path, so e.g. to 
get access to QString, which is located under QtCore/, you'd typically just 
write:

  #include 

This is the recommended, documented way of pulling in headers.
Though you could also write:

  #include 

as the parent parent directory is also in the list of include paths.

Looking at the code, it seems we have access to the parent directory, so we 
could do that name check (which I suppose has less potential for false 
positives than checking the file name).
For framework builds, the directory would be "Headers", which also seems safe.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

> the files are located in include directories

These are directories that **may** contain headers, not directories that 
**only** contain headers. (Which we mostly expect `-Isystem` to be).
For example, many projects keep headers next to sources, and so have sources on 
their include path. And the directory containing the current file is always on 
the include path.
We'd expect to see `Makefile`, `SConstruct`, `BUILD` files etc here. And maybe 
a smattering of "random" files that don't follow particular conventions.

Trying to support QT headers seems very reasonable though. Seems like our 
options are:

- current behavior with false negatives on QT
- proposed behavior with false positives on Makefile etc
- current behavior and try to detect QT as an exception
- proposed behavior and try to detect Makefile etc as exceptions

WDYT about detecting QT headers specifically? It seems hacky, but I don't see a 
way out of this that doesn't involve hardcoding some filenames. Are they in a 
directory like `"qt-11/QFoo"` that we can recognize? Even Q followed by another 
capital letter might be a good enough heuristic.
(The docs suggest it's just `` but the docs also say to use angle 
brackets so I'm not sure whether to believe them)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

I hope this isn't too controversial. After all, the files are located in 
include directories, so there shouldn't be any random garbage getting picked up.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112996/new/

https://reviews.llvm.org/D112996

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

[PATCH] D112996: [CodeCompletion] Generally consider header files without extension

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
ckandeler requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Real-world use case: The Qt framework's headers have the same name
as the respective class defined in them, and Qt's traditional qmake
build tool uses -I (rather than -isystem) to pull them in.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112996

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/included-files.cpp


Index: clang/test/CodeCompletion/included-files.cpp
===
--- clang/test/CodeCompletion/included-files.cpp
+++ clang/test/CodeCompletion/included-files.cpp
@@ -22,12 +22,12 @@
 // CHECK-3-NOT: foo.h>
 // CHECK-3: foosys>
 
-// With -I rather than -isystem, the header extension is required.
+// A header extension is not required.
 #include 
 // RUN: %clang -fsyntax-only -I %t/a -Xclang 
-code-completion-at=%t/main.cc:26:13 %t/main.cc | FileCheck 
-check-prefix=CHECK-4 %s
 // CHECK-4-NOT: foo.cc>
 // CHECK-4-NOT: foo.h>
-// CHECK-4-NOT: foosys>
+// CHECK-4: foosys>
 
 // Backslash handling.
 #include "a\foosys"
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9648,7 +9648,8 @@
   if (!(Filename.endswith_insensitive(".h") ||
 Filename.endswith_insensitive(".hh") ||
 Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
+Filename.endswith_insensitive(".inc") ||
+!Filename.contains('.')))
 break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);


Index: clang/test/CodeCompletion/included-files.cpp
===
--- clang/test/CodeCompletion/included-files.cpp
+++ clang/test/CodeCompletion/included-files.cpp
@@ -22,12 +22,12 @@
 // CHECK-3-NOT: foo.h>
 // CHECK-3: foosys>
 
-// With -I rather than -isystem, the header extension is required.
+// A header extension is not required.
 #include 
 // RUN: %clang -fsyntax-only -I %t/a -Xclang -code-completion-at=%t/main.cc:26:13 %t/main.cc | FileCheck -check-prefix=CHECK-4 %s
 // CHECK-4-NOT: foo.cc>
 // CHECK-4-NOT: foo.h>
-// CHECK-4-NOT: foosys>
+// CHECK-4: foosys>
 
 // Backslash handling.
 #include "a\foosys"
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9648,7 +9648,8 @@
   if (!(Filename.endswith_insensitive(".h") ||
 Filename.endswith_insensitive(".hh") ||
 Filename.endswith_insensitive(".hpp") ||
-Filename.endswith_insensitive(".inc")))
+Filename.endswith_insensitive(".inc") ||
+!Filename.contains('.')))
 break;
 }
 AddCompletion(Filename, /*IsDirectory=*/false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits