[PATCH] D156781: Use the canonical path for the include header search.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 1 09:19:42 PDT 2023


sammccall added a comment.

I'm out on vacation the next two weeks, happy to look once back, otherwise @kadircet would be the person to review this on the clangd side.

A couple of general notes:

- people use symlinks in different ways, to give files multiple names, the idea that we can tell which path is "better" is mostly false. Improving one case probably makes another worse.
- at first glance, this seems to change the behavior that FileEntry::getName() starts with the include path as spelled in `-I` etc. This seems like a significant change that is likely to impact a lot of places that may be under-tested, like spellings of filenames in diagnostics.



================
Comment at: clang/lib/Lex/InitHeaderSearch.cpp:179
+    }
+    // If it is a symlink, we add the canonical path.
+    if (auto cDE = FM.getOptionalDirectoryRef(canonical)) {
----------------
Non-equality doesn't mean it's a symlink, some parent could be, or it could simply have been a relative path (can those get here?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156781



More information about the cfe-commits mailing list