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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
