JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.
================
Comment at: lldb/unittests/Target/ExecutionContextTest.cpp:77
+TEST_F(ExecutionContextTest, GetByteOrderTarget) {
+ ArchSpec arch = Target::GetDefaultArchitecture();
+ arch.SetTriple("armv7-pc-apple");
----------------
labath wrote:
> I don't understand this.. Why do you fetch the ArchSpec via
> `Target::GetDefaultArchitecture()` and then immediately overwrite it on the
> next line? Can this line just be deleted?
Yep, I uploaded an old version of the patch. This and the triple should've
already been fixed.
================
Comment at: lldb/unittests/Target/ExecutionContextTest.cpp:124
+ ExecutionContext process_ctx(process_sp);
+ EXPECT_EQ(process_sp->GetByteOrder(), process_ctx.GetByteOrder());
+}
----------------
labath wrote:
> It doesn't look like the presence of a process will change what
> ExecutionContext::GetByteOrder returns, as it checks the target first. OTOH,
> I don't know why would the byte order of a process be ever different from the
> byte order of its target...
Yeah, I think the logic in the implementation is redundant, this is more of a
test to prevent regressions in case the implementation would change in the
future.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D48704/new/
https://reviews.llvm.org/D48704
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
