labath added inline comments.
================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:192
+ packet_result = ReadPacket(
+ response, std::chrono::duration_cast<std::chrono::microseconds>(
+ GetPacketTimeout())
----------------
zturner wrote:
> `using namespace std` is generally frowned upon, but I wonder if we could be
> more lenient about `using namespace std::chrono`? Would make a lot of those
> code easier on the eyes.
I started using it in some tighter scopes, which are chrono-heavy. I'd be fine
with the using directive at file level though.
================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:266-267
+ packet,
+ std::chrono::duration_cast<std::chrono::microseconds>(GetPacketTimeout())
+ .count(),
+ false);
----------------
zturner wrote:
> Same here. You should just be able to pass the `duration` straight through,
> and update `ReadPacket` to do the cast.
These should go away, when everything is updated to use chrono, but the
hierarchy there is multiple levels deep, so I cannot do it in a single pass
(ReadPacket just calls `WaitForPacketWithTimeoutMicroSecondsNoLock`, which
would also need to be updated... the real cast should only happen somewhere in
the Connection classes, once you actually start issuing system calls).
================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h:86
+ ScopedTimeout(GDBRemoteCommunication &gdb_comm,
+ std::chrono::seconds timeout);
~ScopedTimeout();
----------------
zturner wrote:
> To make this more generic, this could be a `std::chrono::duration<int>`. If
> for any reason someone wanted `3.2` seconds, they could then specify it.
I don't think that works the way you think it does. If you don't specify the
ratio template argument, it will just get defaulted to `std::ratio<1>`, meaning
seconds. To have it generic you'd have to make this a template function, with
the ratio as the parameter, but at that point you might as well just choose a
type with the highest precision you are willing to accept. So, I could make
this take a milli or micro second durations, but I don't think that's necessary
for a packet timeout.
https://reviews.llvm.org/D25391
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
