[PATCH] D83834: Add test utility 'extract'

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/tools/extract/no-leading-lines.s:4
+
+# RUN: extract --no-leading-lines input %s | llvm-mc -triple=x86_64 - 2>&1 | FileCheck %s
+
----------------
grimar wrote:
> I wonder if it is better to use a tool that doesn't require "REQUIRES: x86-registered-target".
> E.g. it probably could be yaml2obj that reported a syntax error.
For this one, we can drop -triple=x86_64. `#` in the line beginning is a universal comment among targets.

For others, I want to avoid syntax errors because that would change the exist code of the tool.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/strip-symbol.test:14
+#--- list1
+bar # bar
+baz # baz
----------------
grimar wrote:
> `#` -> `##` while you are here?
Since this is about the input, `#` works better, i.e. the syntax does not require two `##` to start a comment..


================
Comment at: llvm/tools/extract/extract.cpp:63
+          line.substr(markerLen - 4).startswith("--- ")))
+      continue;
+    separator = line.substr(0, markerLen);
----------------
grimar wrote:
> This supports `//---`, but doesn't seem you have a test for it?
> I'd also add a comment saying it tries to find ``^(.|//)--- <part>`` line.
> 
> Perhaps, it is easier to read this piece when it is:
> ```
>      if (line.size() <= markerLen ||
>           !line.substr(markerLen - 4).startswith("--- ")))
>        continue;
> ```
> 
> 
basic.s has `//--- cc`


================
Comment at: llvm/tools/extract/extract.cpp:68
+      if (partBegin) {
+        error(input, "'" + separator + cur + "' occurs more than once");
+        exit(1);
----------------
grimar wrote:
> test?
basic.s has `# DUP:`


================
Comment at: llvm/tools/extract/extract.cpp:81
+  if (!partBegin)
+    error(input, "'" + separator + part + "' was not found");
+  if (!partEnd)
----------------
grimar wrote:
> test?
basic.s has `was not found`


================
Comment at: llvm/tools/extract/extract.cpp:111
+  if (input.empty())
+    error("input filename is not specified");
+  ErrorOr<std::unique_ptr<MemoryBuffer>> bufferOrErr =
----------------
grimar wrote:
> test?
`basic.s` has `# NO_INPUT:`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83834