[PATCH] D83834: Add test utility 'extract'

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.


================
Comment at: llvm/test/tools/extract/basic.s:2
+# REQUIRES: x86-registered-target
+# RUN: extract aa %s | llvm-mc -triple=x86_64 - 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=AA --implicit-check-not='warning: bb'
----------------
grimar wrote:
> grimar wrote:
> > MaskRay wrote:
> > > jhenderson wrote:
> > > > Do you actually need to use llvm-mc at all? It seems a bit heavy duty. You could just use `FileCheck` directly on the output of `extract`, e.g.
> > > > 
> > > > ```
> > > > # RUN: extract aa %s | FileCheck %s --check-prefix=AA --implicit-check-not=bb
> > > > ...
> > > > 
> > > > # AA: {{^}}aa{{$}}
> > > > 
> > > > #--- aa
> > > > aa
> > > > ```
> > > > 
> > > > I suppose that doesn't cover the line numbering, but perhaps that should be a different test?
> > > I suppose the only problem is `# REQUIRES: x86-registered-target` but this is the simplest tool (I can find) which can print line number information, and we do need to test several forms and ensure each form can get correct line numbers.
> > > 
> > > If you have suggestion for another tool without a need to add `# REQUIRES: x86-registered-target`, I'd happily change. But I'd prefer sticking with llvm-mc otherwise.
> > Perhaps, you could use the LLVM`s `count` tool to count the number of lines?
> Another way I can think of is to add more sample files to compare with.
> I.e. you can create sample.a, sample.b etc. And then compare the output of extract with those samples.
> Will this work? I just also do not think that using of `llvm-mc` is a good idea for the tool that splits the text lines...
> 
> Perhaps, you could use the LLVM`s count tool to count the number of lines?

This seems like an indirect way to test something.

> I.e. you can create sample.a, sample.b etc. And then compare the output of extract with those samples.

This degrades readability. Adding a line means all Inputs/ files need updating.

.warning has well defined and very stable interface. We use nothing but a very basic feature set of lexing of it. No assembly, no disassembly. I don't see a problem using it for testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83834