[PATCH] D123197: Remove a few effectively-unused FileEntry APIs. NFC

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 6 05:16:02 PDT 2022


sammccall added a comment.

In D123197#3432386 <https://reviews.llvm.org/D123197#3432386>, @kadircet wrote:

> thanks lgtm! I would still be looking out for build bot statuses in case there are OS specific code paths that were using isvalid.

Will do!

> it might also be worthwhile to send out `operator<` change in a separate commit, just to give downstream users an opportunity to handle fixes for the two in isolation (or reverts if need be).

I think it's so unlikely to be used as to not be worth it.

- It's not possible to put FileEntrys as values in a container, so some code would have to be explicitly comparing them (rather than std::sort or so).
- Ordering file entries by UID doesn't seem terribly useful - it's neither the cheapest ordering (pointers) nor the most natural.
- Empirically, there are zero uses in llvm-project and zero in google's downstream repo, which is one of the heavier user of clang APIs. I'd bet there are none anywhere.



================
Comment at: clang/lib/Basic/FileManager.cpp:462
   UFE->Dir     = &DirInfo->getDirEntry();
-  UFE->UID     = NextFileUID++;
-  UFE->IsValid = true;
+  UFE->UID = NextFileUID++;
   UFE->File.reset();
----------------
kadircet wrote:
> nit: revert formatting
`arc` insists on this change here, because it's in the blast radius of the next line and clang-format isn't configured to allow this formatting.

Unless you feel strongly I'd rather not fight the linter when it's not really wrong.
(Happy to reformat the other lines to match though)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123197



More information about the cfe-commits mailing list