rsmith added a comment.
In https://reviews.llvm.org/D37954#884029, @Lekensteyn wrote:
> Any objection for merging these patches? I rebased today (no changes needed)
> and it still passes clang-tests.
This is not the way that we do code review in LLVM and Clang. If you don't get
a response to a patch, you're expected to keep pinging it (typically once a
week) until you do get an answer. This past week in particular has been
especially bad as we had the mailing deadline for the C++ committee meeting on
Monday and the LLVM developer meeting yesterday and today.
I reverted this in r316195, because it breaks users using
`-fno-canonical-prefixes` to request that we do not do this kind of path
canonicalization.
================
Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:967-968
+ if (Arg *A = Args.getLastArg(options::OPT_fno_canonical_system_headers,
+ options::OPT_fcanonical_system_headers)) {
+ if (A->getOption().matches(options::OPT_fno_canonical_system_headers)) {
----------------
Use `hasFlag` instead.
Also, defaulting this on will break existing users who use
`-fno-canonical-prefixes` to turn off the (wrong in general) assumption that
using `realpath` is meaningful. Please default to the value of that flag
instead. I would guess (but haven't checked) that flag controls this behavior
in GCC.
================
Comment at: cfe/trunk/lib/Frontend/DependencyFile.cpp:296
+ if (CanonicalSystemHeaders && isSystem(FileType)) {
+ StringRef RealPath = FE->tryGetRealPathName();
+ if (!RealPath.empty() && RealPath.size() < Filename.size()) {
----------------
This is not an appropriate use of `tryGetRealPathName`. Its purpose is to get
the way the file name was stored in disk (particularly, preserving the original
case on a case-sensitive file system), not to resolve symlinks.
`FileManager::getCanonicalName` would be the right thing to call.
Repository:
rL LLVM
https://reviews.llvm.org/D37954
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
