[GitHub] [arrow] nealrichardson commented on a change in pull request #7901: ARROW-9543: [C++] Simplify parsing/formatting utilities

[GitBox <gi...@apache.org>]

nealrichardson commented on a change in pull request #7901:
URL: https://github.com/apache/arrow/pull/7901#discussion_r508855487



##########
File path: cpp/src/arrow/json/converter.cc
##########
@@ -123,13 +123,11 @@ class NumericConverter : public PrimitiveConverter {
     RETURN_NOT_OK(builder.Resize(dict_array.indices()->length()));
 
     auto visit_valid = [&](string_view repr) {
-      value_type value;
-      if (!internal::ParseValue(numeric_type_, repr.data(), repr.size(), &value)) {
-        return GenericConversionError(*out_type_, ", couldn't parse:", repr);
+      if (auto value = internal::ParseValue(numeric_type_, repr)) {

Review comment:
       This is nice that you fixed this "maybe-undefined" `value`. There are two more like this in `csv/converter.cc`: can you fix those too? cf. https://github.com/r-windows/rtools-packages/runs/1277507717#step:4:893
   
   (How difficult would it be to add a CI job that fails on `-Wmaybe-undefined`? I feel like every time I inspect the C++ build logs on the R mingw builds (which is infrequent), I find more warnings like this.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org