[Lldb-commits] [PATCH] D61292: Include inlined functions when figuring out a contiguous address range

António Afonso via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri May 3 11:40:32 PDT 2019


aadsm marked an inline comment as done.
aadsm added a comment.

@labath thank you for your kind words!



================
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;
+
----------------
labath wrote:
> aadsm wrote:
> > 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`?
> A utility function sounds nice. (a test class would be fine too, but I'd name it a bit less generic, as not all of our unit tests are in business of running yaml2obj and creating modules).
> 
> The part I'm not so sure about is the location. Originally the idea was that we would have a special subfolder for test helpers related to each module under test, but then at some point that got changed into `TestingSupport` which sounds more generic (this evolution here is visible in the fact that the cmake target name in that folder is called `lldbUtilityHelpers`). If we put this function there then we'd have to pull in the Core module (and everything that goes with it). Though that isn't that bad on it's own, it is a bit unfortunate, as right now the `Utility` unit test executable is our best defense against unexpected dependencies creeping into the main module. After this, that executable would link in the whole world again, and we'd lose this defense.
> 
> Another possibility might be to just put the yaml2obj (which is the main source of mess here) part in that file, and leave the construction of the Module object to the users.
Yeah, the way I did it locally was to create a function that only handles the yaml2obj part. Some tests use the file for other things other than creating a ModuleSpec.
I also put the responsibility of creating and cleaning up the object file in the caller. I was not sure how to handle the clean up of the temporary file for all different cases. Here's how it looks like from the caller side.

```
llvm::SmallString<128> obj;
if (auto ec = llvm::sys::fs::createTemporaryFile("source-%%%%%%", "obj", obj))
  return llvm::errorCodeToError(ec);
llvm::FileRemover obj_remover(obj);
if (auto error = ReadYAMLObjectFile("inlined-functions.yaml", obj))
  return llvm::Error(std::move(error));
```
What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61292





More information about the lldb-commits mailing list