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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 4 03:39:55 PST 2020


grimar 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
+
----------------
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.


================
Comment at: llvm/tools/yaml2obj/yaml2obj.cpp:54
+    }
+    Substitution.try_emplace(F, V);
+  }
----------------
```
Substitution[F] = V;
```

`try_emplace` is not needed to be used here, is't? `StringRef` is a cheap type for copying and
`moving` concept does not make sense at all I think.



================
Comment at: llvm/tools/yaml2obj/yaml2obj.cpp:76
+
+  return Preprocessed;
+}
----------------
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());
    }
  }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73821





More information about the llvm-commits mailing list