[PATCH] D48144: [Support] Teach YAMLIO about polymorphic types
Scott Linder via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 9 12:34:19 PDT 2018
scott.linder marked 7 inline comments as done.
scott.linder added a comment.
I can submit a separate patch to clean up things like making the empty definitions into declarations, removing redundant `public:` qualifiers, and removing redundant references to `this`, but I don't want to mix those changes into this patch.
I will update this patch with the other changes, as well as full context.
================
Comment at: include/llvm/Support/YAMLTraits.h:301
+/// };
+template <typename T> struct PolymorphicTraits {
+ // Must provide:
----------------
dblaikie wrote:
> Perhaps this could/should be a declaration rather than a definition? (since it doesn't have any members anyway)
I agree, but I am just following the convention in the file, which is an empty definition (see e.g. MappingContextTraits). I mistakenly uploaded the patch without context, but I will include it in the next revision.
================
Comment at: include/llvm/Support/YAMLTraits.h:552
+
+public:
+ static bool const value = (sizeof(test<PolymorphicTraits<T>>(nullptr)) == 1);
----------------
dblaikie wrote:
> You can drop this since it's the default (all members are public in a struct)
Again I'm trying to follow a convention found throughout the file for `has_` structs, although all of these could be updated to drop the redundant qualifier.
https://reviews.llvm.org/D48144
More information about the llvm-commits
mailing list