aadsm marked 4 inline comments as done and an inline comment as not done.
aadsm added inline comments.
================
Comment at: lldb/unittests/Symbol/TestLineEntry.cpp:60-87
+#define EXPECTED_NO_ERROR(x)
\
+ if (std::error_code ASSERT_NO_ERROR_ec = x) {
\
+ llvm::SmallString<128> MessageStorage;
\
+ llvm::raw_svector_ostream Message(MessageStorage);
\
+ Message << #x ": did not return errc::success.\n"
\
+ << "error number: " << ASSERT_NO_ERROR_ec.value() << "\n"
\
+ << "error message: " << ASSERT_NO_ERROR_ec.message() << "\n";
\
----------------
jingham wrote:
> These defines don't seem specific to this test, is there a more general place
> you could put them?
I could put it in `TestUtilities.h`?
================
Comment at: lldb/unittests/Symbol/TestLineEntry.cpp:96-122
+llvm::Expected<ModuleSP> LineEntryTest::GetModule() {
+ if (m_module_sp)
+ return m_module_sp;
+
+ std::string yaml = GetInputFilePath("inlined-functions.yaml");
+ llvm::SmallString<128> obj;
+
----------------
jingham wrote:
> It looks like this bit of business has been copied around in a bunch of other
> tests in the unittest framework. Could we put this in a common place (like
> make a LLDBUnitTest : testing::Test class that does this?
How about adding a new function to TestUtilities.cpp named `ReadYAMLObject`?
================
Comment at: lldb/unittests/Symbol/TestLineEntry.cpp:193-194
+ int result = sum2(a, b) + sum2(c, d);
+ return result;
+}
+
----------------
jingham wrote:
> Can you make the return here not trivial (like return result + 5)} The way
> you have written the line return line doesn't contribute any code so you
> would step all the way out to the caller, but it would be good to test that
> we didn't just extend the range to our caller, i.e. that next on "int result
> ="... stops at the "return result" which it should do except in your example
> line 13 contributes no code.
That's a good idea, thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61292/new/
https://reviews.llvm.org/D61292
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
