[PATCH] D59142: YAMLIO: Improve template arg deduction for mapOptional

[Pavel Labath via Phabricator via llvm-commits]

labath created this revision.
labath added reviewers: zturner, sammccall.
Herald added subscribers: jdoerfert, kristina.
Herald added a project: LLVM.

The way c++ template argument deduction works, both arguments are used
to deduce the template type in the three-argument overload of
mapOptional. This is a problem if the types are slightly different, even
if they are implicitly convertible. This is fairly easy to trigger with
integral types, as the default type of most integral constants is int,
which then requires casting the constant to the type of the other
argument.

This fixes that by telling the compiler to not use the default value for
argument deduction. The "telling" is done by spelling out the argument
type in a more complicated manner, as deduction only happens for simple
types. This allows the default value to be any type which is implicitly
convertible to the value type.


Repository:
  rL LLVM

https://reviews.llvm.org/D59142

Files:
  include/llvm/Support/YAMLTraits.h
  unittests/Support/YAMLIOTest.cpp


Index: unittests/Support/YAMLIOTest.cpp
===================================================================
--- unittests/Support/YAMLIOTest.cpp
+++ unittests/Support/YAMLIOTest.cpp
@@ -823,7 +823,7 @@
       io.mapRequired("f1", c.f1);
       io.mapRequired("f2", c.f2);
       io.mapRequired("f3", c.f3);
-      io.mapOptional("f4", c.f4, MyFlags(flagRound));
+      io.mapOptional("f4", c.f4, flagRound);
      }
   };
 }
@@ -1327,8 +1327,8 @@
     static void mapping(IO &io, TotalSeconds &secs) {
       MappingNormalization<NormalizedSeconds, TotalSeconds> keys(io, secs);
 
-      io.mapOptional("hours",    keys->hours,    (uint32_t)0);
-      io.mapOptional("minutes",  keys->minutes,  (uint8_t)0);
+      io.mapOptional("hours", keys->hours, 0);
+      io.mapOptional("minutes", keys->minutes, 0);
       io.mapRequired("seconds",  keys->seconds);
     }
   };
Index: include/llvm/Support/YAMLTraits.h
===================================================================
--- include/llvm/Support/YAMLTraits.h
+++ include/llvm/Support/YAMLTraits.h
@@ -863,8 +863,13 @@
     mapOptionalWithContext(Key, Val, Ctx);
   }
 
+  // NB: The std::enable_if is used on the Default argument to prevent this
+  // argument from being used in template parameter deduction. This allows the
+  // default value to be specified without a cast even if it's type does not
+  // exactly match the type of Val.
   template <typename T>
-  void mapOptional(const char *Key, T &Val, const T &Default) {
+  void mapOptional(const char *Key, T &Val,
+                   const typename std::enable_if<true, T>::type &Default) {
     EmptyContext Ctx;
     mapOptionalWithContext(Key, Val, Default, Ctx);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59142.189879.patch
Type: text/x-patch
Size: 1693 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190308/a8d33fc8/attachment.bin>