[libc-commits] [PATCH] D123061: [libc] Add holder class for va_lists

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/__support/vlist_holder.h:17
+
+class VListHolder {
+  va_list vlist;
----------------
lntue wrote:
> Maybe VaListHolder is a bit clearer?
I vote for something even simpler: `ArgList`?


================
Comment at: libc/test/src/__support/vlist_holder_test.cpp:15
+  va_list vlist;
+  va_start(vlist, n);
+  __llvm_libc::internal::VListHolder v(vlist);
----------------
Should the `va_start` and `va_end` be managed by the new class?


================
Comment at: libc/test/src/__support/vlist_holder_test.cpp:20
+  for (int i = 0; i < n; ++i) {
+    v.next_var<int>();
+  }
----------------
Implementing the c++ iterator protocol will help will make the for-loop iteration cleaner?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123061