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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 3 07:16:43 PDT 2021


sammccall added a comment.

In D112996#3105783 <https://reviews.llvm.org/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



More information about the cfe-commits mailing list