[Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like how you've separated out the conversion function doing the actual conversion. That should make it easy to write unit tests for it (including tests for malformed input). Can you add something like that? I am particularly interested in what will the merging code do when it encounters reference loops.

However, I find the usage of shared_ptr in the parsing code overkill. Surely we can come up with something better when the lifetime of all of the objects is local to the translation function. I think the most llvm-y solution would be to use a BumpPtrAllocator to create all nodes, and then just dispose of that when the function returns. (this assumes the nodes are trivially destructible, which it looks like they will be after they stop storing shared_ptrs).



================
Comment at: source/Plugins/SymbolFile/PDB/PDBFPOProgramToDWARFExpression.cpp:203
+private:
+  const std::map<llvm::StringRef, FPOProgramNodePtr> &m_dependent_programs;
+};
----------------
use `llvm::DenseMap` ?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55122