[libc-commits] [PATCH] D144145: [libc] add mock arg list

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/__support/arg_list.h:36
 
+// Used for testing things that use an ArgList.
+class MockArgList {
----------------
Add a comment explaining under what contexts this is useful.


================
Comment at: libc/src/__support/arg_list.h:42
+  LIBC_INLINE MockArgList() = default;
+  LIBC_INLINE MockArgList(va_list vlist) {
+    // Copy the vlist to suppress "unused parameter" warnings.
----------------
Remove the argname `vlist` here, then you shouldn't need the copy to suppress unused parameter warnings.


================
Comment at: libc/src/__support/arg_list.h:60
+    ++arg_counter;
+    return T(arg_counter);
+  }
----------------
Since we don't care about the return value, this can be just `T()`.


================
Comment at: libc/src/__support/arg_list.h:66
+
+#ifndef LIBC_COPT_MOCK_ARG_LIST
+using ArgProvider = ArgList;
----------------
I think this can be improved to make it cleaner but to keep the scope of this change small, let us just do one more thing here: Move this to `parser.h` so that only the class `Parser` has to worry about it.


================
Comment at: libc/src/stdio/fprintf.cpp:24
   va_start(vlist, format);
-  internal::ArgList args(vlist); // This holder class allows for easier copying
-                                 // and pointer semantics, as well as handling
-                                 // destruction automatically.
+  internal::ArgProvider args(vlist); // This holder class allows for easier
+                                     // copying and pointer semantics, as well
----------------
Why should this and others outside of the class `Parser` change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144145