[PATCH] D138019: [ARM][AArch64] De-template TargetParser types

[Sam Elliott via Phabricator via llvm-commits]

lenary added a comment.

Some comments. The namespace one is more important than the plural-ness comments.



================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.h:86
 
-template <typename T> struct ArchNames {
+struct ArchNames {
   const char *NameCStr;
----------------
is this in a different namespace to the arm version of this struct?

also, why does this class have a plural name? It doesn't represent a list, it represents an entry in a list? is this something we can fix now?


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.h:135
 // When this becomes table-generated, we'd probably need two tables.
-template <typename T> struct CpuNames {
+struct CpuNames {
   const char *NameCStr;
----------------
I think this also needs to be in a namespace so it doesn't collide with the ARM variant?

I have similar issues with the pluralness of this class name.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138019/new/

https://reviews.llvm.org/D138019