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

[Pavel Labath via Phabricator via llvm-commits]

labath marked an inline comment as done.
labath added inline comments.


================
Comment at: include/llvm/Support/YAMLTraits.h:896
                               Context &Ctx) {
-    this->processKeyWithDefault(Key, Val, Default, false, Ctx);
+    const T &DefaultAsT = Default; // Use only implicit conversions.
+    this->processKeyWithDefault(Key, Val, DefaultAsT, false, Ctx);
----------------
zturner wrote:
> Another possibility is `static_assert(std::is_convertible<T, DefaultT>::value, "Default type must be implicitly convertible to value type!");`  which should have the same effect, and also yield a better compiler error message.  Then, since explicit conversions are disallowed by the `static_assert`, you could re-write the call site as `this->processKeyWithDefault(Key, Val, static_cast<T>(Default), false, Ctx);`
> 
> I don't have a strong preference, but this feels mildly better to me.  WDYT?
Yes, that does look better. I'll update the patch with that.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59142