Page MenuHomePhabricator

Correct include suggestion when search path includes symlink
Needs ReviewPublic

Authored by puremourning on Aug 14 2019, 1:54 PM.

Details

Reviewers
sammccall
Summary

When a header search path contained a symlink, the suggested include
spelling could be wrong, as the files to insert provided by clangd are
always canonical (i.e. with the symlinks removed), but the search paths
(which come from flags), might not be. As files from Sema also come from
the VFS, they are also canonical, I think.

When trying to suggest the spelling of the include to insert, we should
always compare canonical paths. This corrects the include suggestion in
this case.

Event Timeline

puremourning created this revision.Aug 14 2019, 1:54 PM

FYI this fixes the issue in https://github.com/clangd/clangd/issues/124

I haven't added new tests to HeaderSearchTest.cc because the InMemoryFileSystem doesn't support symlinks and I didn't want to try and implement that for this patch. Let me know thoughts on alternative ways to regression test it.

Tests?

Per my comment: I haven't added new tests to HeaderSearchTest.cc because the InMemoryFileSystem doesn't support symlinks and I didn't want to try and implement that for this patch. Let me know thoughts on alternative ways to regression test it.

Tests?

Per my comment: I haven't added new tests to HeaderSearchTest.cc because the InMemoryFileSystem doesn't support symlinks and I didn't want to try and implement that for this patch. Let me know thoughts on alternative ways to regression test it.

OK I found test/SemaCXX/missing-header

Might be able to use the _real_ filesystem and go via the diagnostic rather than clangd