simark added inline comments.
================
Comment at: lib/Basic/FileManager.cpp:319
- SmallString<128> RealPathName;
- if (!FS->getRealPath(InterndFileName, RealPathName))
- UFE.RealPathName = RealPathName.str();
+ if (UFE.File) {
+ if (auto Path = UFE.File->getName()) {
----------------
ioeric wrote:
> simark wrote:
> > What's the rationale for only computing the field if `UFE.File` is non-null?
> >
> > Previously, if you looked up the file with `openFile == false` and then
> > later `openFile == true`, the `RealPathName` field would not be set because
> > of this. That doesn't seem right.
> There has been no guarantee that RealFilePath is always set. I think that's
> the reason why the acceasor is called tryGetRealPathName.
The way I understood it was that it could be empty because computing the real
path can fail. Not just because we didn't skipped computing it.
================
Comment at: lib/Basic/FileManager.cpp:326
+ llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+ UFE.RealPathName = AbsPath.str();
+ }
----------------
ioeric wrote:
> simark wrote:
> > If the path contains symlinks, doesn't this put a non-real path in the
> > RealPathName field? Won't users (e.g. clangd) use this value thinking it
> > is a real path, when it is actually not?
> This was the original behavior. In general, File Manager should never call
> real_path for users because it can be very expensive. Users should call
> real_path if they want to resolve symlinks. That said, it's fair to say that
> "RealPathName" is just a wrong name, and we should clean it up at some point.
Ok, then if the goal is not to actually have a real path (in the realpath(3)
sense), that's fine. But I think we should rename the field sooner than later,
it's really confusing.
That also means that it's kind of useless for us in clangd, so we should always
call real_path there and not rely on that field.
Repository:
rC Clang
https://reviews.llvm.org/D51159
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
