JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM modulo comments



================
Comment at: lldb/include/lldb/Target/RegisterFlags.h:21-22
+  public:
+    Field(llvm::StringRef name, unsigned start, unsigned end)
+        : m_name(name.str()), m_start(start), m_end(end) {
+      assert(m_start <= m_end && "Start bit must be <= end bit.");
----------------
If you're going to store the string anyway, you might as well take it by value 
and move it into the member. That'll save you a copy in case someone calls the 
ctor with an rvalue reference. 


================
Comment at: lldb/include/lldb/Target/RegisterFlags.h:26
+
+    // Get size of the field in bits. Will always be at least 1.
+    unsigned GetSizeInBits() const { return m_end - m_start + 1; }
----------------
This applies to the rest of this file as well.


================
Comment at: lldb/include/lldb/Target/RegisterFlags.h:72
+  // Gaps are allowed, they will be filled with anonymous padding fields.
+  RegisterFlags(llvm::StringRef id, unsigned size,
+                const std::vector<Field> &fields);
----------------
Same here


================
Comment at: lldb/include/lldb/Target/RegisterFlags.h:81-83
+  std::string m_id;
+  // Size in bytes
+  unsigned m_size;
----------------
Looks like these can be const?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145566

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to