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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
