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

Reply via email to