sammccall added a comment.
Code looks good apart from one case where we encounter
Foo.framework/Subdir1/Subdir2.
Can you add a test to `clang/test/CodeCompletion`? `included-files.cpp` has the
existing tests, not sure if you can add them there or it needs to be an obj-c
file for framework includes.
================
Comment at: lib/Sema/SemaCodeComplete.cpp:8404
+ if (LookupType == DirectoryLookup::LT_Framework) {
+ if (!Filename.endswith(".framework")) {
+ break;
----------------
nit: no {} needed for these one-line `if`s
================
Comment at: lib/Sema/SemaCodeComplete.cpp:8404
+ if (LookupType == DirectoryLookup::LT_Framework) {
+ if (!Filename.endswith(".framework")) {
+ break;
----------------
sammccall wrote:
> nit: no {} needed for these one-line `if`s
this check appears to be incorrect, it'll fire even if NativeRelDir is nonempty
(i.e. a we're inside a framework already.)
I think this can be combined with the one below it, see next comment.
================
Comment at: lib/Sema/SemaCodeComplete.cpp:8407
+ }
+ if (NativeRelDir.empty()) {
+ Filename.consume_back(".framework");
----------------
`consume_back` checks for the suffix, just `if (NativeRelDir.empty() &&
!Filename.consume_back(".framework")) break;` is enough
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58062/new/
https://reviews.llvm.org/D58062
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
