[Lldb-commits] [PATCH] D48704: [ExecutionContext] Return the target/process byte order.

[Jonas Devlieghere via Phabricator via lldb-commits]

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