[libcxx-commits] [PATCH] D96986: [libc++] Drop template layer when using vsnprintf

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius added subscribers: mstorsjo, curdeius.
curdeius added a comment.

Just a few comments from a quick look.



================
Comment at: libcxx/include/__config:1452
 
+#ifdef __GNUC__
+#  define _LIBCPP_PRINTFILE(fmtarg, firstvararg) \
----------------
Does clang-cl support this attribute? If yes, then it should be `#if defined(__GNUC_) || defined(__clang__)`.


================
Comment at: libcxx/include/__config:1453
+#ifdef __GNUC__
+#  define _LIBCPP_PRINTFILE(fmtarg, firstvararg) \
+     __attribute__((__format__(__printf__, fmtarg, firstvararg)))
----------------
The macro name is misleading for me. It doesn't print, and it doesn't print files. It annotates a printf-like function though.
IMO it should contain something like format and attribute in the name, but I haven't given it a long thought.


================
Comment at: libcxx/src/filesystem/directory_iterator.cpp:275
     if (m_ec)
-      err.report(m_ec, "at root \"%s\"", root);
+      err.report(m_ec, "at root \"%s\"", root.c_str());
   }
----------------
@mstorsjo, you probably need to sync on this part regarding https://reviews.llvm.org/D98077.


================
Comment at: libcxx/src/filesystem/filesystem_common.h:78
+  GuardVAList args_guard(args);
+  // we might need a second shot at this, so pre-emptivly make a copy
   va_list args_cp;
----------------
Typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96986