zturner added a comment.

I thought about what my "ideal" design would look like.  I'm not suggesting 
anyone should actually go off and do this, but since we're brainstorming design 
anyway, it doesn't hurt to consider what an optimal one should look like (and 
for the record, there may be things I haven't considered that cuase this design 
to not work or be suboptimal or not work at all).  Just throwing it out there.

1. Rename `ObjectFile` to `ModuleFile` to indicate that a) it makes no sense 
without a Module, and b) it doesn't make sense for symbol-only files.

2. Rename `SymbolVendor` to `SymbolFileLocator`.  Mostly cosmetic, but more 
clearly differentiates the responsibilities of the two classes.  (e.g. wouldn't 
you expect SymbolFiles to be able to "vend" symbols)?

3. `SymbolFileLocator` returns a `unique_ptr<SymbolFile>`.  Nothing more.  It 
has no other methods.

4. `Target` contains a `vector<unique_ptr<SymbolFileLocator>>` as well as 
methods `AddSymbolFileLocator(unique_ptr<SymbolFileLocator>)` and 
`unique_ptr<SymbolFile> LocateSymboleFile(Module&)`.  The former gives things 
like the Platform or Process the ability to customize the behavior.  The 
latter, when called, will iterate through the list of `SymbolFileLocators`, 
trying to find one that works.

5. `Module` contains a `unique_ptr<SymbolFile>`.  When `GetSymbolFile()` is 
called for the first time, it calls `Target::LocateSymbolFile(*this)` and 
updates its member variable.

6. `ModuleFile` is updated to support "capabilities" much like `SymbolFile` is 
today.  One of these capabilities can be "has unwind info".  Likewise, "has 
unwind info is added to `SymbolFile` capabilities as well.  `Module` can then 
select the best provider of unwind info using this approach.

And finally

7. `SymbolFile` now stores a variable `m_module` instead of `m_obj_file` since 
this is usually what the `SymbolFile` requires anyway, and it also eliminates 
the dependency on `SymbolFile` being backed by an `ObjectFile`.  Note that in 
this case, a `SymbolFileBreakpad` for example would have its `m_module` 
variable set to the actual module that its providing symbols for.


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

https://reviews.llvm.org/D55142



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to