[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