labath added inline comments.
================
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:74
+struct TraceIntelPTGetStateResponse : TraceGetStateResponse {
+ /// `nullptr` if no tsc conversion rate exists.
+ llvm::Optional<TimestampCounterRateSP> tsc_rate;
----------------
wallace wrote:
> avoid using nullptr when you have an Optional.
Ideally this would be a plain `TimestampCounterRateSP` variable and nullptr
would denote emptyness. Since (shared) pointers already have a natural empty
state, it's best to use that. Adding an extra Optional layer just creates
ambiguity.
================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:22
#include "llvm/ADT/None.h"
+#include <memory>
----------------
wallace wrote:
> jj10306 wrote:
> >
> memory should be defined as a new include block between lines 9 and 11.
> That's just the pattern we follow
It's actually a pattern we should not follow. When all of the includes are
grouped in one block, then clang-format automatically reorders them to conform
to the [[https://llvm.org/docs/CodingStandards.html#include-style | llvm coding
standards ]].
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120595/new/
https://reviews.llvm.org/D120595
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
