I see the analogy to Python's range(), except that it returns a sequence of numbers, not a list. But I'm not really too worried about this.

Nice new tests. Thanks!

I've added two reviewers. Could I get a second opinion on the name of this bang operator?

I'm tempted to suggest that this bang operator should be called !rangelist. The name !range suggests a pair of integers or a sequence of integers not encapsulated in a list. Add a couple of other reviewers so we can get a second opinion.

Thanks for doing this.

Yes, that makes sense.

The TableGen Programmer's Reference does not include descriptions of backend facilities.

I think you have everything covered.

I think my approval is sufficient.

Looks good to me.

I think you can use defvar to define two constants strue and sfalse, then use them instead of true and false. However, defvar'ed constants are a bit funky, so you'll have to test it.

The 'true' and 'false' literals are available.

You are good to go.

Sorry I've been scarce for this patch. Doesn't it need documentation?

Sorry, I'm pretty busy now and don't have time right away to work on this. Would it make sense to document it now? Then we can determine whether the two schemes can be merged in a reasonable way.

Why don't you take an inventory of the number of uses of GenericEnum? Then we can see if it's practical to replace those uses.

So one possibility is to deprecate GenericEnum, replace all uses of it with your Enum, and document Enum as the standard solution.

Sorry, I'm just quickly passing through for now. How does this new feature relate to the existing GenericEnum class?

Looks good to me.

Sorry, I don't know what the CI test suite is. I don't know much about the testing facilities.

Did you run all the TableGen tests?

ProgRef.rst looks good to me.

Thanks for making these corrections to the TableGen document. See my comments above.

The change to ProgRef.rst looks good to me. The other two changes look fine, though I do not "own" those files.

Doesn't C++ allow unnamed parameters? That would be another way to do it. And that would make it impossible to reference the template argument name.

I mean something really explicit, like a statement especially designed for this.

I vote for something explicit. An unused field seems like a hack, no?

I suggested an 'unused' statement above. With it, we should be able to make this option on by default.

@c-rhodes Oh, absolutely, no need for a new statement until we determine whether there are legitimate reasons for unused template arguments. In fact, if we determine that there are none, we could make the new option on by default.

I would propose something radical like a new statement:

(I've been scarce due to summer holidays and, sadly, real work.)

Sorry, I removed it but didn't update this review. All set now.

Could I get an LGTM on this? I have removed the spurious comment.

One must amend the commit when one makes changes to the files.

The arm_*.inc files do not change with this revision.

I couldn't find anything in the actions to "restart review." Did I miss it?

I'll start a new Phabrication for the change.

I reverted this commit. There is at least one build that uses a 'sed' that interprets \x00 as "x00" and so does not substitute a NUL character.

I used David's trick to eliminate the NUL characters in the test file. They are now represented by at signs (@) and translated on the fly.

LGTM.

Hang on, there is already a TableGen test that uses sed: intrin-properties.td. So obviously I have sed in the right place.

No, I was confused, thinking that the modified file would go back through Lit. But of course it doesn't. I will set up something like you suggest. I'm sure there is a way to use an escape sequence to represent the NUL character in sed . . . yes, \000.

There are test files that use sed to do replacements. I'll ask on llvm dev how to create a RUN: line that does the right thing. It has to replace @ with NUL and get rid of the sed command so it doesn't recurse forever. My Unix command knowledge is meager.

Okay, sounds good. I'll see what I can find.

There are four or five NULs in the test file, to check them in various positions. Would it be helpful to add a second nul-char.txt file that is a duplicate, except that it has an at sign (@) where every null is? I could refer to it with a comment in nul-char.td.

The TableGen frontend has no way of knowing whether a particular backend, will use a record, nor any way of knowing whether no backends at all will use it. I'm not sure how we could detect unused records.

LGTM. Just please verify that it passes all the TableGen tests.

Nice cleanup! I presume this passes all the TableGen tests?

I added a test. There are NULs in the .td file.

Restored whitespace as requested by Jan.

Now the //// comments should be gone.

Hmm. I swore I deleted all the //// lines. Hang on . . .

I will push this on Wednesday.

@paperchalice: I don't understand the purpose of this revision. Could you explain in more detail?

It makes good sense to spend some time now on additional TableGen file cleanup. There is one file with TODOs for assert, so I will start there.

I added !find() to complement !substr(), so that TableGen would have the two most common string functions. I believe !find will be helpful with 'assert' as people begin adding assertions to TableGen files.

Incorporate latest comments from @dblaikie and @craig.topper.