thakis added a comment.
I went ahead and landed this with the comments requested by Duncan in r324385.
(http://llvm.org/viewvc/llvm-project?view=revision&revision=324385)
I'm also quoting Duncan's email reply to my comment, since that's not showing
up on phab:
"""
Using runtime availability che
(To be clear, the patch as-is is 100% fine with me.)
On Thu, Feb 1, 2018 at 11:44 AM, Duncan P. N. Exon Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Using runtime availability checking doesn't make sense for a system
> Libc++, as you point out. If we add runtime checks they ought
Using runtime availability checking doesn't make sense for a system Libc++, as
you point out. If we add runtime checks they ought to be non-default, and
hidden behind configuration flags.
Also, do I remember correctly that __builtin_available requires linking against
Foundation (and thus the O
thakis added a comment.
The powers that be updated the SDK on the chromium clang bots, so we need some
solution for this issue soon.
The approach in this patch should work, but it seems conservative. Now libc++
only uses utimesat() if it's built with a deployment target of macOS 10.13. But
if
mclow.lists added inline comments.
Comment at: src/experimental/filesystem/operations.cpp:27
+// We can use the presence of UTIME_OMIT to detect platforms that do not
+// provide utimensat, with some exceptions on OS X.
+#if !defined(UTIME_OMIT) ||
EricWF added inline comments.
Comment at: src/experimental/filesystem/operations.cpp:23-28
+// We can use the presence of UTIME_OMIT to detect platforms that do not
+// provide utimensat, with some exceptions on OS X.
+#if !defined(UTIME_OMIT) || \
+ (defined(__MAC_OS_X_VERSION
dexonsmith added inline comments.
Comment at: src/experimental/filesystem/operations.cpp:22-24
+#if defined(__APPLE__)
+#include
+#endif
EricWF wrote:
> dexonsmith wrote:
> > I only just noticed you were including Availability.h. That shouldn't be
> > necessar
EricWF added inline comments.
Comment at: src/experimental/filesystem/operations.cpp:22-24
+#if defined(__APPLE__)
+#include
+#endif
dexonsmith wrote:
> I only just noticed you were including Availability.h. That shouldn't be
> necessary, since the macros shou
dexonsmith added inline comments.
Comment at: src/experimental/filesystem/operations.cpp:23-28
+// We can use the presence of UTIME_OMIT to detect platforms that do not
+// provide utimensat, with some exceptions on OS X.
+#if !defined(UTIME_OMIT) || \
+ (defined(__MAC_OS_X_VER
dexonsmith added inline comments.
Comment at: src/experimental/filesystem/operations.cpp:23-28
+// We can use the presence of UTIME_OMIT to detect platforms that do not
+// provide utimensat, with some exceptions on OS X.
+#if !defined(UTIME_OMIT) || \
+ (defined(__MAC_OS_X_VER
EricWF updated this revision to Diff 102783.
EricWF marked an inline comment as done.
EricWF added a comment.
- Address inline comments.
https://reviews.llvm.org/D34249
Files:
src/experimental/filesystem/operations.cpp
Index: src/experimental/filesystem/operations.cpp
==
EricWF updated this revision to Diff 102782.
https://reviews.llvm.org/D34249
Files:
src/experimental/filesystem/operations.cpp
Index: src/experimental/filesystem/operations.cpp
===
--- src/experimental/filesystem/operations.cpp
+
EricWF added inline comments.
Comment at: src/experimental/filesystem/operations.cpp:23-28
+// We can use the presence of UTIME_OMIT to detect platforms that do not
+// provide utimensat, with some exceptions on OS X.
+#if !defined(UTIME_OMIT) || \
+ (defined(__MAC_OS_X_VERSION
dexonsmith added a comment.
This is the right idea, although it only covers macOS.
Any ideas for how to test this?
Comment at: src/experimental/filesystem/operations.cpp:23-28
+// We can use the presence of UTIME_OMIT to detect platforms that do not
+// provide utimensat, with
EricWF created this revision.
This fixes llvm.org/PR33469.
https://reviews.llvm.org/D34249
Files:
src/experimental/filesystem/operations.cpp
Index: src/experimental/filesystem/operations.cpp
===
--- src/experimental/filesystem/
15 matches
Mail list logo
