[libcxx-commits] [libcxx] [libcxx] Cache file attributes during directory iteration. (PR #93316)

[Louis Dionne via libcxx-commits]

================
@@ -201,7 +201,11 @@ class directory_entry {
     _IterNonSymlink,
     _RefreshSymlink,
     _RefreshSymlinkUnresolved,
-    _RefreshNonSymlink
+    _RefreshNonSymlink,
+#  if _LIBCPP_AVAILABILITY_HAS_FILESYSTEM_FULLY_POPULATED_CACHED_ENTRY
----------------
ldionne wrote:

That's an interesting point.

I was also going to ask if there's a reason why you can't reuse the existing `_IterSymlink` and `_IterNonSymlink` constants. IIUC that's because the old dylib populates the cache incompletely and then sets `_IterSymlink` as the cache type, correct? So if you reused `_IterSymlink` you'd have no way of knowing whether the cache data is coming from an old dylib that half-populated the entry or if it came from a new dylib that populated it fully. I guess that was the first approach you had tried in your original patch.

So then that leaves two options:
1. Reuse `_IterSymlink` and `_IterNonSymlink` but also conditionalize the use of certain bits of data on `_LIBCPP_AVAILABILITY_HAS_FILESYSTEM_FULLY_POPULATED_CACHED_ENTRY`, since that tells you whether they were populated properly.
2. Introduce new cache entry types (`_IterCachedSymlink` & friends) that represent whether the entry has been fully populated. Since those are added at the end of the enum, you wouldn't need the availability macro in this case since code running against an old dylib would only ever see the old cache types.

Do we have the same understanding of the situation and tradeoffs?

If so, my preference would be to go for (1), where we introduce an availability macro and reuse the existing cache type. The reason is that we can eventually get rid of the availability macros (in a long time, but eventually) and it clearly documents that we did something that's ABI-affecting here. Adding new enum values at the end and using them unconditionally is clever and it should work IIUC, but it doesn't leave a clear trace and we'll never really clean that up.

https://github.com/llvm/llvm-project/pull/93316