[libc] Add holder class for va_lists

[michaelrj]

Summary

This class is intended to be used in cases where a class is being used
on a va_list. It provides destruction and copy semantics with small
overhead. This is intended to be used in printf.

[lntue]

libc/src/__support/vlist_holder.h

17 ↗

(On Diff #420268)

Maybe VaListHolder is a bit clearer?

libc/test/src/__support/vlist_holder_test.cpp

1 ↗

(On Diff #420268)

Need to be updated.

[sivachandra]

libc/src/__support/vlist_holder.h

17 ↗

(On Diff #420268)

I vote for something even simpler: ArgList?

libc/test/src/__support/vlist_holder_test.cpp

15 ↗

(On Diff #420268)

Should the va_start and va_end be managed by the new class?

20 ↗

(On Diff #420268)

Implementing the c++ iterator protocol will help will make the for-loop iteration cleaner?

[michaelrj]

rename to ArgList and fix formatting

libc/src/__support/vlist_holder.h

17 ↗

(On Diff #420268)

I went with ArgList since it's shorter.

libc/test/src/__support/vlist_holder_test.cpp

15 ↗

(On Diff #420268)

The class handles its own va_end, but I don't think it can handle the va_start since its constructor is a new function and you can only create a va_list for the scope of the current function. I could probably create a macro to handle this invisibly, but that's as close as we can get, I think.

20 ↗

(On Diff #420268)

I don't think an iterator works here, since the iterator needs to have a type, and the values may be of various types. In addition, there's no end marker or length in a va_list, meaning that a for loop won't know when to end.

[sivachandra]

libc/test/src/__support/vlist_holder_test.cpp

15 ↗

(On Diff #420268)

OK. I am not very confident about how va_list is implemented. For example, calling va_end in a destructor is also not OK as it is a different stack frame. OK for now.

20 ↗

(On Diff #420268)

Ah, yes! Sorry for the noise.

libc/src/__support/CMakeLists.txt

 )
 
 add_header_library(
   integer_operations
   HDRS
     integer_operations.h
 )
 
+add_header_library(
+  arg_list
+  HDRS
+    arg_list.h
+)
+
 # Thread support is used by other support libraries. So, we add the "threads"
 # before other directories.
 add_subdirectory(threads)
 
 add_subdirectory(File)
 add_subdirectory(FPUtil)
 add_subdirectory(OSUtil)

libc/src/__support/arg_list.h

+//===-- Holder Class for manipulating va_lists ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_SUPPORT_ARG_LIST_H
+#define LLVM_LIBC_SRC_SUPPORT_ARG_LIST_H
+
+#include <stdarg.h>
+
+namespace __llvm_libc {
+namespace internal {
+
+class ArgList {
+  va_list vlist;
+
+public:
+  ArgList(va_list vlist) { va_copy(this->vlist, vlist); }
+  ArgList(ArgList &other) { va_copy(this->vlist, other.vlist); }
+  ~ArgList() { va_end(this->vlist); }
+
+  template <class T> T inline next_var() { return va_arg(vlist, T); }
+};
+
+} // namespace internal
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_SUPPORT_ARG_LIST_H

libc/test/src/__support/CMakeLists.txt

add_libc_unittest(
   SUITE
     libc_support_unittests
   SRCS
     str_to_float_test.cpp
   DEPENDS
     libc.src.__support.str_to_float
 )
 
+add_libc_unittest(
+  arg_list_test
+  SUITE
+    libc_support_unittests
+  SRCS
+    arg_list_test.cpp
+  DEPENDS
+    libc.src.__support.arg_list
+)
+
 add_executable(
   libc_str_to_float_comparison_test
   str_to_float_comparison_test.cpp
 )
 
 target_link_libraries(libc_str_to_float_comparison_test
   PRIVATE
     llvmlibc

libc/test/src/__support/arg_list_test.cpp

+//===-- Unittests for ArgList ---------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/arg_list.h"
+
+#include "utils/UnitTest/Test.h"
+
+int get_nth_int(int n, ...) {
+  va_list vlist;
+  va_start(vlist, n);
+  __llvm_libc::internal::ArgList v(vlist);
+  va_end(vlist);
+
+  for (int i = 0; i < n; ++i) {
+    v.next_var<int>();
+  }
+  return v.next_var<int>();
+}
+
+TEST(LlvmLibcArgListTest, BasicUsage) {
+  ASSERT_EQ(get_nth_int(5, 0, 10, 20, 30, 40, 50, 60, 70, 80, 90), 50);
+}
+
+int sum_two_nums(int first, int second, ...) {
+  va_list vlist;
+  va_start(vlist, second);
+  __llvm_libc::internal::ArgList v1(vlist);
+  va_end(vlist);
+
+  __llvm_libc::internal::ArgList v2 = v1;
+
+  int first_val;
+  for (int i = 0; i < first; ++i) {
+    v1.next_var<int>();
+  }
+  first_val = v1.next_var<int>();
+
+  int second_val;
+  for (int i = 0; i < second; ++i) {
+    v2.next_var<int>();
+  }
+  second_val = v2.next_var<int>();
+
+  return first_val + second_val;
+}
+
+TEST(LlvmLibcArgListTest, CopyConstructor) {
+  ASSERT_EQ(sum_two_nums(3, 1, 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024),
+            10);
+
+  ASSERT_EQ(sum_two_nums(3, 5, 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024),
+            40);
+}