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

[David Spickett via Phabricator via llvm-commits]

DavidSpickett accepted this revision.
DavidSpickett added a comment.

> Searching for +xyz will actually lead you to where it is defined.

Sure, that makes sense (especially in the current state where the `.def` is the only documentation).



================
Comment at: llvm/lib/Support/AArch64TargetParser.cpp:110
   for (const auto &AE : AArch64ARCHExtNames)
-    if (AE.Feature && ArchExt == AE.getName())
-      return StringRef(AE.Feature);
+    if (!AE.Feature.empty() && ArchExt == AE.Name)
+      return AE.Feature;
----------------
tmatheson wrote:
> DavidSpickett wrote:
> > Maybe this is just me but "!empty()" could just be "size()".
> > 
> > I know sometimes containers make empty cheaper but here it's just `== 0` vs `!= 0`.
> For checking that "the feature is not the empty string", `!empty()` makes a lot more sense to me semantically than an implicit `cast<bool>(size())`. I'm trying to move away from the C-style code :) I would hope (haven't checked) that they compile to basically the same thing.
Agree to disagree, keep it as you prefer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138026