[PATCH] D75015: [ARM] Rewrite ARMAttributeParser

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 25 01:56:28 PST 2020


grimar added a comment.

In D75015#1888821 <https://reviews.llvm.org/D75015#1888821>, @samparker wrote:

> This is a large change and converting to lower case is making really hard to discern what the functional changes are.


+1. It does not feel right that mixing of code style and function changes happens.



================
Comment at: llvm/lib/Support/ARMAttributeParser.cpp:501
+    } else if (Error e = parseAttributeList(size))
+      return e;
   }
----------------
Seems it can be:

```
if (sw) {
  DictScope scope(*sw, scopeName);
  if (!indicies.empty())
    sw->printList(indexName, indicies);
} 

if (Error e = parseAttributeList(size))
  return e;
```


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2691
 
-    ARMAttributeParser(&W).parse(Contents, support::little);
+    // TODO Print error and delete the redundant FormatVersion check above.
+    if (Error E = ARMAttributeParser(&W).parse(Contents, support::little)) {
----------------
`TODO:`


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2694
+      consumeError(std::move(E));
+    }
   }
----------------
Excessive curly braces around a single line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75015





More information about the llvm-commits mailing list