labath added a comment. In D74759#1896100 <https://reviews.llvm.org/D74759#1896100>, @unnar wrote:
> In D74759#1895748 <https://reviews.llvm.org/D74759#1895748>, @labath wrote: > > > That is a fair point. I suppose the reason why I see this as different is > > because this field is an implementation detail of the RangeDataVector > > class, and so the user should not even see it -- whereas the user has a > > legitimate reason to at least access the other fields (and most of the > > methods only provide read-only access to these fields). > > > > I'm sorry, I haven't gotten around to looking at this patch today, but I > > thought I'd at least say that... > > > That is true. I am fine with changing it if that's the only thing that you > see as blocking this change from passing. I finally took a good look at the patch, and I think that is the only remaining question on my mind. Could you try implementing that to see how it looks like? @teemperor, do you have any more thoughts on this? ================ Comment at: lldb/include/lldb/Utility/RangeMap.h:814 + size_t mid = (lo + hi) / 2; + auto &entry = m_entries[mid]; + ---------------- [[ http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable | Llvm style guide ]] does not recommend using `auto` in this situation. `Entry &entry` is one character longer, but it makes it clear what is going on. ================ Comment at: lldb/include/lldb/Utility/RangeMap.h:834 + size_t mid = (lo + hi) / 2; + auto entry = m_entries[mid]; + ---------------- `const Entry &` -- http://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74759/new/ https://reviews.llvm.org/D74759 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
