[PATCH] D21785: [RFC]Add unittests to AArch64TargetParser

Saleem Abdulrasool via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 28 09:52:07 PDT 2016


compnerd added inline comments.

================
Comment at: include/llvm/Support/TargetParser.h:152
@@ +151,3 @@
+#include "AArch64TargetParser.def"
+  AAK_LAST
+};
----------------
I think that you should use the `AK_` prefix as is the norm.  That said, you could do something else which IMO is better: use `enum class` and drop the prefix.

================
Comment at: unittests/Support/TargetParserTest.cpp:24
@@ -21,3 +23,3 @@
 #undef AARCH64_ARCH
 };
 
----------------
Please delete this.  I had added it to avoid hardcoding the offset.  Just iterate over the enumeration below.  In fact, it is unused.

================
Comment at: unittests/Support/TargetParserTest.cpp:68
@@ +67,3 @@
+#undef AARCH64_CPU_NAME
+};
+
----------------
This set really feels weird to me.


http://reviews.llvm.org/D21785





More information about the llvm-commits mailing list