zturner added inline comments.
================
Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:21
+
+ LLVM_BUILTIN_DEBUGTRAP;
+ delay = false;
----------------
This will work on MSVC and presumably clang. I'm not sure about gcc. Is that
sufficient for your needs? Do you know if gcc has the `__builtin_debugtrap`
intrinsic?
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:83-88
+ if (endian == LITTLE) {
+ registers[register_id] = SwitchEndian(value_str);
+ }
+ else {
+ registers[register_id] = stoul(value_str, nullptr, 16);
+ }
----------------
jmajors wrote:
> zturner wrote:
> > This code block is unnecessary.
> >
> > ```
> > unsigned long X;
> > if (!value_str.getAsInteger(X))
> > return some error;
> > llvm::support::endian::write(®isters[register_id], X, endian);
> > ```
> >
> > By using llvm endianness functions you can just delete the `SwitchEndian()`
> > function entirely, as it's not needed.
> These endian functions don't work here. getAsInteger() assumes the number is
> in human reading order (i.e. big endian). So it's converting the little
> endian string as if it's big, then converting that little endian long to
> another little endian long, which does nothing.
> I need something that reverses the string first.
>
> What do you think about adding a version of StringRef::getAsInteger that
> accepts an endianness parameter?
Hmm.. What about this?
```
unsigned long X;
if (!value_str.getAsInteger(X))
return some error;
sys::swapByteOrder(X);
```
It shouldn't matter if you swap the bytes in the string before doing the
translation, or swap the bytes in the number after doing the translation should
it?
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:64
+public:
+ static std::shared_ptr<StopReply> Create(const llvm::StringRef response,
+ llvm::support::endianness endian);
----------------
Can you constructor argument does not need to be `const`, as `StringRefs` are
immutable by definition.
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:69
+protected:
+ explicit StopReply();
+
----------------
Don't need `explicit` if there are no arguments. It's mostly useful for single
argument constructors.
https://reviews.llvm.org/D32930
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
