[llvm] [llvm] Add format check for MCSubtargetFeatures (PR #180943)

[David Spickett via llvm-commits]

DavidSpickett wrote:

If you're going to do this check, I agree that createMCSubtargetInfo is the logical place to do that.

Idk if you've finished updating the PR description, but since I was looking anyway...

> At this moment feature string isn't checked for fitting in such format. This leads to assertion failure

So we can:
* Parse the string more leniently (at least for sensible cases), or -
* Add this strict format check and document exactly how strict it is.

I need to see, in the PR description, a justification for the choice between those two.

(I'm not pushing one or the other right now, just want to have it for future reference)

> With implementing additional format check we can avoid such problems.

>From the perspective of the caller of this API it does not avoid the problems, it surfaces the problem earlier. It's no less cryptic but at least it's early. If I tracked this nullptr return in a debugger, it's better that it's in a factory function rather than in some unrelated callstack accessing a member of MCSubtargetInfo.

So in that sense, it's more actionable, but I still think we need better documentation, and some form of trail from this new failure mode to that documentation.

An improvement could be:
```
 /// Features are encoded as a string of the form 
 ///   "+attr1,+attr2,-attr3,...,+attrN" 
 /// <...>
 /// The string must match exactly that format otherwise construction of <whatever> will fail.
```
And it is probably easier to show the string in some sort of grammar notation rather than listing every possible violation. Because some of the things are subtle, like trailing comma not being allowed, I would not get that right on the first attempt. Keep the current string as an example input though, that's nice to have next to a more formal grammar or regex whatever you want to use.

> such problems

Please include an example of one such problem. Like:
```
For example feature string "abcnbasdfsdfsdfskdjfsdfkj" is accepted, and results in feature list "sdfds sdf sdfsdf sdf sdf sdf". Later in whateverClass, it asserts that <some property holds>.
```

Apart from anything else, when my downstream tools break, at least I will be able to be grateful that I'm being protected from problems I didn't know I could have.

https://github.com/llvm/llvm-project/pull/180943