[PATCH] D62838: [Object] add isExecutableObject member function

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/unittests/Object/ObjectFileTest.cpp:76
+
+// I have ommited a scope creating do { } while(0) for this macro because the
+// scope destroys these RAII objects causing tests to fail. Do not do
----------------
Don't use first person in comments. If you feel you need a comment, rephrase this as "Omit a scope..." etc, or "This macro omits a scope creating..."

It would also be easier to read this comment if you surround code snippets with quotation marks.


================
Comment at: llvm/unittests/Object/ObjectFileTest.cpp:79
+// something like if(...) createObjFile;. This couldn't be a function because
+// gtest macros return something.
+#define createObjFile(Ptr, Header, Magic)                                      \
----------------
What do they return? What's stopping your function returning something and then asserting in the code on that? E.g:

```
std::string my_function(bool b) {
  if (b)
    return "";
  return "something went wrong";
}

TEST(a_test) {
  auto result = my_function(somecall());
  ASSERT_TRUE(result.empty()) << result;
  ...
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62838/new/

https://reviews.llvm.org/D62838