[PATCH] D51918: [Support] sys::fs::directory_entry includes the file_type.

[Sam McCall via Phabricator via llvm-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.


================
Comment at: llvm/trunk/lib/Support/Unix/Path.inc:704
+  // The DTTOIF macro lets us reuse our status -> type conversion.
+#if defined(_DIRENT_HAVE_D_TYPE) && defined(DTTOIF)
+  return typeForMode(DTTOIF(Entry->d_type));
----------------
jordan_rose wrote:
> Picking this up a year later by noticing that `_DIRENT_HAVE_D_TYPE` isn't actually defined in macOS's headers, which means it probably isn't in BSD either. Do you think just testing for `DTTOIF` would be sufficient?
Oops, I didn't realize that while `d_type` is a BSD extension that GNU picked up, `_DIRENT_HAVE_D_TYPE` seems to be a GNU extension that BSD doesn't have :-\

> Do you think just testing for DTTOIF would be sufficient?
That sounds plausible to me, there isn't much point defining `DTTOIF` if you don't have d_type to pass to it.

That sounds plausible to me. And the "good news" is that it would fail with a noisy compile error.
So I think the worst case is the patch you propose ends up getting reverted.

I guess the "proper" way to do this is to have CMake detect this and create a `HAVE_DIRENT_D_TYPE` macro in `config.h`. But I have no objection to trying the simple thing first.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D51918