sammccall added inline comments.
================ Comment at: clang/lib/Basic/FileManager.cpp:318 FileEntryRef ReturnedRef(*NamedFileEnt); - if (UFE.isValid()) { // Already have an entry with this inode, return it. + if (ReusingEntry) { // Already have an entry with this inode, return it. ---------------- dexonsmith wrote: > I'm trying to figure out why `ReusingEntry` was necessary. It looks to me > like you could just call `UFE->isValid()` here and avoid introducing that > `bool`... or am I missing something? You're right, but I think we can get rid of isValid, and plan to send a patch. A FileEntry handed out by FileManager always has isValid = true. It's not possible to create a useful FileEntry outside FileManager (and this is never done outside FileManagerTest). So the concept of validity isn't needed. (IMO the main cost of having the isValid flag is conceptual complexity - lots of places check it, which means people must be thinking about how to handle this case) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123144/new/ https://reviews.llvm.org/D123144 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits