[PATCH] D83834: Add test utility 'extract'

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 3 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'
----------------
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.


================
Comment at: llvm/tools/extract/extract.cpp:26
+
+static cl::OptionCategory cat("extract Options");
+
----------------
jhenderson wrote:
> MaskRay wrote:
> > jhenderson wrote:
> > > Is there a way of telling clang-tidy to allow a different naming style?
> > Added `.clang-tidy` (like lldb/.clang-tidy and lld/.clang-tidy)
> I'm not seeing the `.clang-tidy` in the file list?
It is in the file list: `A	M		llvm/tools/extract/.clang-tidy (19 lines)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83834