MrTrillian marked an inline comment as done.
MrTrillian added inline comments.
================
Comment at: clang/lib/Basic/FileManager.cpp:663
+ } else {
+ llvm::sys::path::remove_dots(AbsPathBuf, /*remove_dot_dot=*/true);
+ CanonicalName = AbsPathBuf.str().copy(CanonicalNameStorage);
----------------
benlangmuir wrote:
> Removing .. can change where the path points in the presence of symlinks; is
> this needed?
> Removing .. can change where the path points in the presence of symlinks; is
> this needed?
@benlangmuir That's true and not ideal, but `makeAbsolute` will not resolve
`/./` or `/../` in paths, so it's not a canonicalization and some tests were
failing because of that. One alternative would be to use `makeAbsolute` +
`remove_dots` on Windows (where removing dot dots is semantically correct) and
`getRealPath` on Unix, like I do in lit. Suggestions?
================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:190
- StringRef FileName = File->tryGetRealPathName().empty()
- ? File->getName()
- : File->tryGetRealPathName();
+ StringRef FileName = SM.getFileManager().getCanonicalName(File);
----------------
benlangmuir wrote:
> Why is this change needed?
> Why is this change needed?
@benlangmuir We don't want raw `getRealPath`s on Windows because of substitute
drives and MAX_PATH issues. That is the idea behind this diff. If I leave the
`tryGetRealPathName` here, I need to change the `relative_include.m` test as in
this previous diff: https://reviews.llvm.org/D154130?id=539683 , which is
undesirable.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154130/new/
https://reviews.llvm.org/D154130
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
