[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
Wed Feb 5 23:53:57 PST 2020


grimar added a comment.

OK. I have 2 non-strong comments/suggestions about the current code.



================
Comment at: llvm/tools/yaml2obj/yaml2obj.cpp:54
+                                        yaml::ErrorHandler ErrHandler) {
+  DenseMap<StringRef, StringRef> Defines;
+  for (StringRef Define : D) {
----------------
Should we exit early for a general case?

```
  if (D.empty())
    return Buf.str();
```

It might simplify things for those who want to debug the code.


================
Comment at: llvm/tools/yaml2obj/yaml2obj.cpp:74
+      auto It = Defines.find(Macro);
+      if (It != Defines.end() && Buf.substr(I).startswith("]]")) {
+        Preprocessed += It->second;
----------------
Here is a little logical issue. If we have input like "[[AAA[",
then you still assign "AAA" to `Macro` and perform a lookup,
but at fact "AAA" is not a macro and you do not need to do a lookup for it.


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