[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

This revision was automatically updated to reflect the committed changes. Closed by commit rG6ea45e3007b8: [lldb] Add RegisterFlags class (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145566/new/ https://reviews.llvm.org

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

DavidSpickett updated this revision to Diff 512692. DavidSpickett added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145566/new/ https://reviews.llvm.org/D145566 Files: lldb/include/lldb/Target/RegisterFlags.h lldb/source/Ta

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM 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-commi

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

DavidSpickett updated this revision to Diff 506902. DavidSpickett added a comment. Use LOG instead of LOGF. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145566/new/ https://reviews.llvm.org/D145566 Files: lldb/include/lldb/Target/RegisterFlags.

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

DavidSpickett updated this revision to Diff 506882. DavidSpickett added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145566/new/ https://reviews.llvm.org/D145566 Files: lldb/include/lldb/Target/RegisterFlags.

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

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)

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

DavidSpickett updated this revision to Diff 504104. DavidSpickett added a comment. Fix typo, remove `m_type` which is not used by the following patches. We could use it down the road, but we can also just guess that single bit fields are bools and anything else is a number. So we don't lose much.

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. LGTM. I'm not really advocating to changing `m_type` to be an enum, but curious how that might fit into the total context of this feature, and if it makes any sense. ==

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

DavidSpickett updated this revision to Diff 503378. DavidSpickett marked an inline comment as done. DavidSpickett added a comment. Add newline at end of file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145566/new/ https://reviews.llvm.org/D14556

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

DavidSpickett updated this revision to Diff 503374. DavidSpickett added a comment. Use StringRef for parameters. This is what the XML parser gives you by default in any case. Internally we want to keep a copy of the string because the XML document will go away after parsing. Repository: rG LL

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

tschuett added a comment. My only comment is: views look more modern than const-ref. The efficiency is not affected at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145566/new/ https://reviews.llvm.org/D145566 ___

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

DavidSpickett added inline comments. Comment at: lldb/include/lldb/Target/RegisterFlags.h:20 + public: +Field(const std::string &name, unsigned start, unsigned end, + const std::string &type) DavidSpickett wrote: > tschuett wrote: > > There are a lo

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

DavidSpickett added inline comments. Comment at: lldb/include/lldb/Target/RegisterFlags.h:20 + public: +Field(const std::string &name, unsigned start, unsigned end, + const std::string &type) tschuett wrote: > There are a lot of `const std::string&

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

tschuett added inline comments. Comment at: lldb/include/lldb/Target/RegisterFlags.h:20 + public: +Field(const std::string &name, unsigned start, unsigned end, + const std::string &type) There are a lot of `const std::string& ` . Are you allowed to

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

tschuett added inline comments. Comment at: lldb/unittests/Target/RegisterFlagsTest.cpp:125 +} \ No newline at end of file Missing line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145566/new/ https://reviews.l

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

DavidSpickett created this revision. Herald added a subscriber: kristof.beyls. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This models the "flags" node from GDB's target XML: https://sourceware