[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