Skip to content

Commit 4b1a509

Browse files
committed
YAMLIO: Improve template arg deduction for mapOptional
Summary: 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 patch fixes that by using a separate template type for the default value, which is then cast to the type of the first argument. To avoid this conversion triggerring conversions marged as explicit, we use static_assert to check that the types are implicitly convertible. Reviewers: zturner, sammccall Subscribers: kristina, jdoerfert, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D59142 llvm-svn: 356157
1 parent 6bc3a77 commit 4b1a509

File tree

2 files changed

+11
-8
lines changed

2 files changed

+11
-8
lines changed

llvm/include/llvm/Support/YAMLTraits.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -863,8 +863,8 @@ class IO {
863863
mapOptionalWithContext(Key, Val, Ctx);
864864
}
865865

866-
template <typename T>
867-
void mapOptional(const char *Key, T &Val, const T &Default) {
866+
template <typename T, typename DefaultT>
867+
void mapOptional(const char *Key, T &Val, const DefaultT &Default) {
868868
EmptyContext Ctx;
869869
mapOptionalWithContext(Key, Val, Default, Ctx);
870870
}
@@ -890,10 +890,13 @@ class IO {
890890
this->processKey(Key, Val, false, Ctx);
891891
}
892892

893-
template <typename T, typename Context>
894-
void mapOptionalWithContext(const char *Key, T &Val, const T &Default,
893+
template <typename T, typename Context, typename DefaultT>
894+
void mapOptionalWithContext(const char *Key, T &Val, const DefaultT &Default,
895895
Context &Ctx) {
896-
this->processKeyWithDefault(Key, Val, Default, false, Ctx);
896+
static_assert(std::is_convertible<DefaultT, T>::value,
897+
"Default type must be implicitly convertible to value type!");
898+
this->processKeyWithDefault(Key, Val, static_cast<const T &>(Default),
899+
false, Ctx);
897900
}
898901

899902
private:

llvm/unittests/Support/YAMLIOTest.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -823,7 +823,7 @@ namespace yaml {
823823
io.mapRequired("f1", c.f1);
824824
io.mapRequired("f2", c.f2);
825825
io.mapRequired("f3", c.f3);
826-
io.mapOptional("f4", c.f4, MyFlags(flagRound));
826+
io.mapOptional("f4", c.f4, flagRound);
827827
}
828828
};
829829
}
@@ -1327,8 +1327,8 @@ namespace yaml {
13271327
static void mapping(IO &io, TotalSeconds &secs) {
13281328
MappingNormalization<NormalizedSeconds, TotalSeconds> keys(io, secs);
13291329

1330-
io.mapOptional("hours", keys->hours, (uint32_t)0);
1331-
io.mapOptional("minutes", keys->minutes, (uint8_t)0);
1330+
io.mapOptional("hours", keys->hours, 0);
1331+
io.mapOptional("minutes", keys->minutes, 0);
13321332
io.mapRequired("seconds", keys->seconds);
13331333
}
13341334
};

0 commit comments

Comments
 (0)