This is an archive of the discontinued LLVM Phabricator instance.

clang/lib/Sema/SemaCodeComplete.cpp
8780	I think we can state the problem more directly here: To know whether a symlink should be treated as file or a directory, we have to stat it. This is cheap enough as there shouldn't be many symlinks. (I think we can drop the heuristic idea from the comment, it was a silly premature optimization)
8784	nit: expand auto here, "type" is vague and `It` is already auto
8786	nit: inline (`if (auto FileStatus = ...)`) drop braces around inner if

This revision is now accepted and ready to land.Feb 18 2020, 4:36 PM

Fixes for Sam's comments

Harbormaster completed remote builds in B46813: Diff 245415.Feb 19 2020, 8:27 AM

In D74790#1881827, @sammccall wrote:

Thanks! I can't see this being a performance problem (famous last words...)

Technically it is capped at 2500 symlinks =) fingers crossed

Closed by commit rGf50fe5eb6d2e: [Sema][CodeComplete] Handle symlinks for include code completion (authored by dgoldman). · Explain WhyFeb 19 2020, 8:51 AM

This revision was automatically updated to reflect the committed changes.

`The test you added in this change seems to be failing on Windows, can you take a look?

http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/14409

FAIL: Clang :: CodeCompletion/included-symlinks.cpp (1779 of 16867)
******************** TEST 'Clang :: CodeCompletion/included-symlinks.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   rm -rf C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp && mkdir -p C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj && mkdir -p C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links
: 'RUN: at line 2';   touch C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h && ln -s C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foo.h
: 'RUN: at line 3';   touch C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h && ln -s C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foobar.h
: 'RUN: at line 4';   touch C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj/test.h && ln -s C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/myproj
: 'RUN: at line 8';   c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe -fsyntax-only -IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links -Xclang -code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:7:13 C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp | c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe -check-prefix=CHECK-1 C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
: 'RUN: at line 14';   c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe -fsyntax-only -IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links -Xclang -code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:13:13 C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp | c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe -check-prefix=CHECK-2 C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
--
Exit Code: 1

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "rm" "-rf" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp"
$ "mkdir" "-p" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj"
$ "mkdir" "-p" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links"
$ ":" "RUN: at line 2"
$ "touch" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h"
$ "ln" "-s" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foo.h"
$ ":" "RUN: at line 3"
$ "touch" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h"
$ "ln" "-s" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foobar.h"
$ ":" "RUN: at line 4"
$ "touch" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj/test.h"
$ "ln" "-s" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/myproj"
$ ":" "RUN: at line 8"
$ "c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe" "-fsyntax-only" "-IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links" "-Xclang" "-code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:7:13" "C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp"
$ "c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe" "-check-prefix=CHECK-1" "C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp"
# command stderr:
C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:9:13: error: CHECK-1: expected string not found in input

// CHECK-1: foo.h"

            ^

<stdin>:1:1: note: scanning from here

COMPLETION: Pattern : fontsub.h"

^

<stdin>:1:27: note: possible intended match here

COMPLETION: Pattern : fontsub.h"

                          ^


error: command failed with exit status: 1

--

In D74790#1883466, @dyung wrote:

`The test you added in this change seems to be failing on Windows, can you take a look?

http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/14409

FAIL: Clang :: CodeCompletion/included-symlinks.cpp (1779 of 16867)
******************** TEST 'Clang :: CodeCompletion/included-symlinks.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   rm -rf C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp && mkdir -p C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj && mkdir -p C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links
: 'RUN: at line 2';   touch C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h && ln -s C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foo.h
: 'RUN: at line 3';   touch C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h && ln -s C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foobar.h
: 'RUN: at line 4';   touch C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj/test.h && ln -s C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/myproj
: 'RUN: at line 8';   c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe -fsyntax-only -IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links -Xclang -code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:7:13 C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp | c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe -check-prefix=CHECK-1 C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
: 'RUN: at line 14';   c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe -fsyntax-only -IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links -Xclang -code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:13:13 C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp | c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe -check-prefix=CHECK-2 C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
--
Exit Code: 1

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "rm" "-rf" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp"
$ "mkdir" "-p" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj"
$ "mkdir" "-p" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links"
$ ":" "RUN: at line 2"
$ "touch" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h"
$ "ln" "-s" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foo.h"
$ ":" "RUN: at line 3"
$ "touch" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h"
$ "ln" "-s" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foobar.h"
$ ":" "RUN: at line 4"
$ "touch" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj/test.h"
$ "ln" "-s" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj" "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/myproj"
$ ":" "RUN: at line 8"
$ "c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe" "-fsyntax-only" "-IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links" "-Xclang" "-code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:7:13" "C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp"
$ "c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe" "-check-prefix=CHECK-1" "C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp"
# command stderr:
C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:9:13: error: CHECK-1: expected string not found in input

// CHECK-1: foo.h"

            ^

<stdin>:1:1: note: scanning from here

COMPLETION: Pattern : fontsub.h"

^

<stdin>:1:27: note: possible intended match here

COMPLETION: Pattern : fontsub.h"

                          ^


error: command failed with exit status: 1

--

Looks like I need to add a // REQUIRES: shell to the test, will submit a fix shortly

dgoldman mentioned this in rG7d91633a2b9b: Fix broken test on Windows caused by D74790.Feb 19 2020, 1:59 PM

Submitted https://github.com/llvm/llvm-project/commit/7d91633a2b9b1f563dc14c632cc0c461c3651f76

Revision Contents

Path

Size

clang/

lib/

Sema/

SemaCodeComplete.cpp

11 lines

test/

CodeCompletion/

included-symlinks.cpp

15 lines

Diff 245428

clang/lib/Sema/SemaCodeComplete.cpp

This file is larger than 256 KB, so syntax highlighting is disabled by default.

Show First 20 Lines • Show All 8,770 Lines • ▼ Show 20 Lines	auto AddFilesFromIncludeDir = [&](StringRef IncludeDir,

std::error_code EC;		std::error_code EC;
unsigned Count = 0;		unsigned Count = 0;
for (auto It = FS.dir_begin(Dir, EC);		for (auto It = FS.dir_begin(Dir, EC);
!EC && It != llvm::vfs::directory_iterator(); It.increment(EC)) {		!EC && It != llvm::vfs::directory_iterator(); It.increment(EC)) {
if (++Count == 2500) // If we happen to hit a huge directory,		if (++Count == 2500) // If we happen to hit a huge directory,
break; // bail out early so we're not too slow.		break; // bail out early so we're not too slow.
StringRef Filename = llvm::sys::path::filename(It->path());		StringRef Filename = llvm::sys::path::filename(It->path());
switch (It->type()) {
		// To know whether a symlink should be treated as file or a directory, we
		sammccallUnsubmitted Done Reply Inline Actions I think we can state the problem more directly here: To know whether a symlink should be treated as file or a directory, we have to stat it. This is cheap enough as there shouldn't be many symlinks. (I think we can drop the heuristic idea from the comment, it was a silly premature optimization) sammccall: I think we can state the problem more directly here: To know whether a symlink should be…
		// have to stat it. This should be cheap enough as there shouldn't be many
		// symlinks.
		llvm::sys::fs::file_type Type = It->type();
		if (Type == llvm::sys::fs::file_type::symlink_file) {
		sammccallUnsubmitted Done Reply Inline Actions nit: expand auto here, "type" is vague and `It` is already auto sammccall: nit: expand auto here, "type" is vague and `It` is already auto
		if (auto FileStatus = FS.status(It->path()))
		Type = FileStatus->getType();
		sammccallUnsubmitted Done Reply Inline Actions nit: inline (`if (auto FileStatus = ...)`) drop braces around inner if sammccall: nit: inline (`if (auto FileStatus = ...)`) drop braces around inner if
		}
		switch (Type) {
case llvm::sys::fs::file_type::directory_file:		case llvm::sys::fs::file_type::directory_file:
// All entries in a framework directory must have a ".framework" suffix,		// All entries in a framework directory must have a ".framework" suffix,
// but the suffix does not appear in the source code's include/import.		// but the suffix does not appear in the source code's include/import.
if (LookupType == DirectoryLookup::LT_Framework &&		if (LookupType == DirectoryLookup::LT_Framework &&
NativeRelDir.empty() && !Filename.consume_back(".framework"))		NativeRelDir.empty() && !Filename.consume_back(".framework"))
break;		break;

AddCompletion(Filename, /IsDirectory=/true);		AddCompletion(Filename, /IsDirectory=/true);
▲ Show 20 Lines • Show All 103 Lines • Show Last 20 Lines

clang/test/CodeCompletion/included-symlinks.cpp

This file was added.

				// RUN: rm -rf %t && mkdir -p %t/real/myproj && mkdir -p %t/links
				// RUN: touch %t/real/foo.h && ln -s %t/real/foo.h %t/links/foo.h
				// RUN: touch %t/real/foobar.h && ln -s %t/real/foobar.h %t/links/foobar.h
				// RUN: touch %t/real/myproj/test.h && ln -s %t/real/myproj %t/links/myproj

				// Suggest symlinked header files.
				#include "foo.h"
				// RUN: %clang -fsyntax-only -I%t/links -Xclang -code-completion-at=%s:7:13 %s \| FileCheck -check-prefix=CHECK-1 %s
				// CHECK-1: foo.h"
				// CHECK-1: foobar.h"

				// Suggest symlinked folder.
				#include "mypr"
				// RUN: %clang -fsyntax-only -I%t/links -Xclang -code-completion-at=%s:13:13 %s \| FileCheck -check-prefix=CHECK-2 %s
				// CHECK-2: myproj/

This is an archive of the discontinued LLVM Phabricator instance.

[Sema][CodeComplete] Handle symlinks for include code completionClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents

Diff 245428

clang/lib/Sema/SemaCodeComplete.cpp

clang/test/CodeCompletion/included-symlinks.cpp

[Sema][CodeComplete] Handle symlinks for include code completion
ClosedPublic