[Lldb-commits] [PATCH] D56564: [SymbolFile] Make ParseCompileUnitXXX accept a CompileUnit&.

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In D56564#1353514 <https://reviews.llvm.org/D56564#1353514>, @clayborg wrote:

> All the change to the symbol vendor make sense, but it seems like all of the call sites should be:
>
>   cu->GetLanguage();
>   cu->ParseFunctions();
>   cu->GetLineTable();
>   cu->ParseDebugMacros();
>   cu->GetSupportFiles();
>   cu->ParseTypes();
>
>
> Some of these calls might already be there, but is seems like we should initiate these calls from the CompileUnit class.


Some functions like that exist today (e.g. `cu->GetLineTable()`, `cu->GetSupportFiles()`, etc), but some don't (e.g. `cu->GetFunctions()` and `cu->GetTypes()` don't exist).  I agree it would be nice to have the `CompileUnit` support all of these, but it might not be purely NFC, or at least it might be easy to accidentally make it *not* be NFC, because we'd be adding some member variables to the `CompileUnit` class for the laziness, to make the implementations match up with the implementation of the existing ones.  Do you think it would be reasonable to start with this patch, which is definitely NFC, and then iterate on it to get more access to these things from the `CompileUnit` class directly?


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

https://reviews.llvm.org/D56564