[PATCH] D74790: [Sema][CodeComplete] Handle symlinks for include code completion

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

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



================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:8780
+
+      // We need to manually resolve symlinks since the directory_iterator
+      // doesn't do it for us. Alternatively we could use a heuristic such as
----------------
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)


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:8784
+      // symlinks.
+      auto Type = It->type();
+      if (Type == llvm::sys::fs::file_type::symlink_file) {
----------------
nit: expand auto here, "type" is vague and `It` is already auto


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:8786
+      if (Type == llvm::sys::fs::file_type::symlink_file) {
+        auto FileStatus = FS.status(It->path());
+        if (FileStatus) {
----------------
nit: inline (`if (auto FileStatus = ...)`)
drop braces around inner if


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74790/new/

https://reviews.llvm.org/D74790