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