[Lldb-commits] [PATCH] D17449: Handle the case when a variable is only valid in part of the enclosing scope

[Tamas Berghammer via lldb-commits]

tberghammer added a comment.

We have 2 (+1) independent dimension here:

1. List of ranges where the variable is in scope
2. List of ranges where the variable is available
3. Format of the location list (it is more of an implementation details because of a format change in dwarf5)

To represent all of the possible combination for a variable we have to handle all dimension separately. I think we have several options:

- Treat the scope as a top level parameter of a variable (same way as we treat the enclosing lexical block) the way I implemented it and if we are worrying about the memory implication then a possible optimization is to say that an empty location list means that the scope is the same as the enclosing block (This way we will add 2 pointer to each Variable in the general case what will be a negligible size increase).
- Make DWARFExpression store 2 list of ranges (1 for the locations and 1 for the scope). I think it is a very similar implementation then the current one but with storing the data in a wrong location so I am strongly against this option.
- Try to make DWARFExpression smart enough to handle the 2 layer of the location expressions (I think this is you suggestion). I think to implement this we need the following changes:
  - Change DWARFExpression to store a RangeMap<lldb::addr_t, lldb::addr_t, DataExtractor> for the location list instead of just a DataExtractor because we need some data format what can be modified instead of the current implementation where DWARFExpression stores only a DataExtractor referencing a blob of data.
  - Teach SymbolFileDWARF about the details of the DWARFExpression format so it can modify it.
  - Abuse the DWARFExpression a bit so it can return some additional value next to the result of the evaluation for the DWARFExpression.

Based on the steps I listed above I don't think it is reasonable to merge the scope information into the DWARFExpression especially if we consider that dwarf expressions are used for several other thing then describing the location of variables. If we don't want to add the scoping information to the Variable class then we can go with a plan like this (I don't really like it):

- Create a new DWARFVariableLocationDescription class
- Remove all location list support from the DWARFExpression class so it really just evaluates dwarf expressions
- In the new class store a map from address range to (dwarf expression + bitfield) where the bitfield value tell if the variable is in/out of scope and if it has/hasn't have location information

I think in one side this would be a fairly clean abstraction and it would also simplify the DWARFExpression class but on the other side I am almost certain then it would have more memory and performance impact then my current implementation (especially if the memory optimization I mentioned is applied) because it will have to create a range map from each dwarf expression and store it in memory.

All in all I don't really share your concern regarding memory impact, but I am happy to optimize it to the point where we use only 3 extra pointer (1 std::vector) compared to the original situation in the common (C/C++) case. Looking into the other options I don't think any of them is any better in terms of memory/performance and they will complicate the use of dwarf expressions all over lldb (the DWARFVariableLocationDescription class would make things cleaner but I don't see any good way to implement it without a higher memory penalty then my current implementation)


http://reviews.llvm.org/D17449