[PATCH] D138026: [ARM][AArch64] Use StringRef in TargetParser structs

[Tomas Matheson via Phabricator via llvm-commits]

tmatheson added inline comments.


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:20-21
 // List is organised as armv8a -> armv8n-a, armv9a -> armv9m-a and armv8-r.
-AARCH64_ARCH("invalid", INVALID, "",
+AARCH64_ARCH("invalid", INVALID, "+",
              AArch64::AEK_NONE)
+AARCH64_ARCH("armv8-a", ARMV8A, "+v8a",
----------------
pratlucas wrote:
> Having the lone `"+"` as arch_feature here looks a bit off. Could we remove the "invalid" entries from the def file completely?
> Would it make sense to use the `INVALID` enum entries by themselves to handle unsuccessful parsing, or maybe use optional returns on the relevant methods?
It does indeed look weird, but that's what it is already doing internally so there is NFC. This patch is already quite large so refactoring the invalid entries would be best done in a follow up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138026