[PATCH] D79810: [FileCheck] Fix isalpha/isalnum calls

[Joel E. Denny via Phabricator via llvm-commits]

jdenny marked an inline comment as done.
jdenny added a comment.

In D79810#2033794 <https://reviews.llvm.org/D79810#2033794>, @thopre wrote:

> It seems I'm responsible for this mess since I introduced the first such patterns. My apologies for not paying enough attention to the type in the API.


Well, it seems no one who reviewed spotted it, and I doubt I would have thought of it either.  :-)



================
Comment at: llvm/test/FileCheck/bad-char.txt:33-36
+ERR-EMPTY-CHECK: error: found empty check string
+ERR-NO-CHECK: error: no check strings found
+ERR-BAD-VAR: error: invalid variable name
+ERR-BAD-STRING-VAR: error: invalid name in string variable definition
----------------
jhenderson wrote:
> Nit: I have a marginal preference for where CHECKs like this aren't shared to interleave them with the RUN lines, i.e.
> 
> ```
> RUN: ... --check-prefix=CHECK1
> 
> CHECK1: ...
> 
> RUN: ... --check-prefix=CHECK2
> 
> CHECK2: ...
> ```
> 
> It makes it slightly easier to follow each individual case. I might be inclined to move the input similarly, but I don't feel strongly about that one either way.
I usually do that and originally wrote this one that way too.  However, due to the symmetry among the cases here, I decided the way I have it now would be easier to read.

But it seems I'm outvoted two to one, and it don't feel strongly about it, especially given that my opinion might flip if the test evolves later.  If no one objects, I'll change to the following form, with horizontal rules to guide the eye:

```
//------------------------------------------------
RUN: ... --check-prefix=CHECK1

input1 lines

CHECK1: ...

//------------------------------------------------
RUN: ... --check-prefix=CHECK2

input2 lines

CHECK2: ...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79810