zturner added inline comments.
================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:56
- response,
-
std::chrono::duration_cast<std::chrono::microseconds>(kInterruptTimeout)
- .count(),
----------------
I think we should all be willing to agree that `using namespace std::chrono` is
fine in CPP files. The namespace is not particularly large, and it makes code
much nicer to read.
================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:249
StringExtractorGDBRemote extra_stop_reply_packet;
- uint32_t timeout_usec = 100000; // 100ms
- ReadPacket(extra_stop_reply_packet, timeout_usec, false);
+ std::chrono::microseconds timeout(100000); // 100ms
+ ReadPacket(extra_stop_reply_packet, timeout, false);
----------------
You can also write `using namespace std::chrono::literals` at the top of the
file, and then you can write:
`ReadPacket(extra_stop_reply_packet, 100ms, false);`
================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:136
#ifdef LLDB_CONFIGURATION_DEBUG
- m_packet_timeout(1000),
+ m_packet_timeout(1000000000),
#else
----------------
clayborg wrote:
> Can me make this clearer with something like:
> ```
> #ifdef LLDB_CONFIGURATION_DEBUG
> m_packet_timeout(std::chrono::seconds(1000));
> #else
> m_packet_timeout(std::chrono::seconds(1));
> #endif
> ```
Or if using literals, `1000s` or `1s`
================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h:83
// created ScopedTimeout object got out of scope
class ScopedTimeout {
public:
----------------
Could this class be templated on duration type? So we could use us, ms, or
whatever pleased us?
================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h:232
PacketResult ReadPacket(StringExtractorGDBRemote &response,
- uint32_t timeout_usec, bool sync_on_timeout);
+ llvm::Optional<std::chrono::microseconds> timeout,
+ bool sync_on_timeout);
----------------
One idea might be to have
```
template <typename T>
using GdbTimeout = llvm::Optional<std::chrono::duration<int, T>
```
then you could write:
```
PacketResult ReadPacket(StringExtractorGDBRemote &response,
GdbTimeout<std::micro> timeout);
```
https://reviews.llvm.org/D26971
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
