[PATCH] D73821: [yaml2obj] Add -D k=v to preprocess the input YAML

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/preprocess.yaml:26
+# RUN: yaml2obj --docnum=2 %s | llvm-nm --just-symbol-name --no-sort - > %t.nosubst
+# RUN: echo -e 'a[[\n[[a\n[[a]\n[[a][[a]][[a]]' | cmp - %t.nosubst
+
----------------
grimar wrote:
> This does not work for me under windows.
> 
> echo -e 'a[[\n[[a\n[[a]\n[[a][[a]][[a]]' | cmp - %t.nosubst
> 
> I see the following when I run this line:
> 
> ```
> D:\Work3\LLVM\llvm-project\build\test\tools\yaml2obj\Output>echo -e 'a[[\n[[a\n[
> [a]\n[[a][[a]][[a]]' | cmp - preprocess.yaml.tmp.nosubst
> - preprocess.yaml.tmp.nosubst различаются: байт 1, строка 1
> ```
> 
> What means that byte 1 at the line 1 is different from the source. (My apologies, I do not know how to
> switch to displaying errors in English for `cmp`).
> 
> I did not dig deeper because anyways I do not like this way of checking symbol names (sorry :),
> so I'd ask you to rewrite in a more traditional way.
Switched to 

```
echo -e '.........' > ...
diff -u ... %t.nosubst
```

Unfortunately we cannot use FileCheck because FileCheck does not like  `[[` which is not used as a FileCheck pattern.


================
Comment at: llvm/tools/yaml2obj/yaml2obj.cpp:76
+
+  return Preprocessed;
+}
----------------
grimar wrote:
> I'd suggest something like the following
> 
> ```
> std::vector<std::pair<std::string, StringRef>> Substitution;
>   for (StringRef Define : D) {
>     StringRef F, V;
>     std::tie(F, V) = Define.split('=');
>     if (!Define.count('=') || F.empty()) {
>       ErrHandler("bad override, missing field name: " + Define);
>       return {};
>     }
>     Substitution.push_back({(Twine("[[") + F + "]]").str(), V});
>   }
> 
>   std::string Document = Buf.str();
>   if (Substitution.empty())
>     return Document;
> 
>   for (std::pair<std::string, StringRef> &P : Substitution) {
>     while (true) {
>       auto I = Document.find(P.first);
>       if (I == std::string::npos)
>         break;
>       Document = Document.replace(I, P.first.size(), P.second.str());
>     }
>   }
> ```
Maybe we should avoid quadratic complexity behavior, just in case that there are a long run of `[[[[[[[[[[`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73821