[PATCH] D95312: [TableGen] [DetailedRecords] Print record name that is null string as ""

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 24 18:28:14 PST 2021


dblaikie added a comment.

In D95312#2518674 <https://reviews.llvm.org/D95312#2518674>, @Paul-C-Anagnostopoulos wrote:

> I think the only test that would make sense is a FileCheck test. Of course, that's the only kind of test I've written, so I might be deluding myself.

Yep, they tend to be the most convenient & pretty good bang-for-buck, etc.

> The problem with a FileCheck test is that it relies on the precise format of the output,

The point of FileCheck is to ensure that the tests are resilient to unrelated format changes (not testing only "is the whole output /exactly/ the same as this example file" (aka: "golden file" tests)) - using regex pattern matches for variable portions, skipping lines unrelated to the particular test/feature, etc.

> which is why the default print records backend of TableGen can never be changed.

We change the output of a variety of tools in LLVM pretty deeply when it's warranted - using sed/regular expressions to mass-update tests is a possibility. Perhaps there are features of these TableGen tests and/or the nature of the output that makes the tests harder to maintain/update than others? Not sure.

> I'd hate to make DetailedRecords' output rigid like that, because it was written precisely to produce a more detailed dump of the TableGen data in a somewhat friendlier format.
>
> I could write a simple test that just checks that all the global variables, classes, and records are at least included in the output, if not their precise format.

I'll leave any extra testing to account for the overall lack of testing here to you - I'm mostly asking for a test for this patch, not to correct historical choices about understesting the general area.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95312



More information about the llvm-commits mailing list